Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test rework, part 3 #3464

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

apostasie
Copy link
Contributor

@apostasie apostasie commented Sep 25, 2024

On top of #3455

Notably:

  • first commit has a raft of evolutions to test tooling in preparation for more test rewrites
  • second commit tentatively addresses flakyness of TestAttach
  • third commit disables parallelism for Docker on prune and rmi tests

@apostasie apostasie changed the title [WIP] Test rework, part 3 Sep 25, 2024
@apostasie apostasie marked this pull request as ready for review September 25, 2024 22:28
@apostasie apostasie marked this pull request as draft September 26, 2024 02:16
@@ -56,54 +56,58 @@ func TestVolumePrune(t *testing.T) {
}

// This set must be marked as private, since we cannot prune without interacting with other tests.
testGroup := &test.Group{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern does not work here, as the Group will run in Parallel with other tests, breaking things for Docker.
It works with nerdctl because we are making the subtests pivate - for Docker, the subtests disable parallel, but since the parent stays parallel, it blows up.

Changing to a Test parent instead of a group, and disabling parallel on the subtests.

@apostasie apostasie force-pushed the qa-tooling-fixes branch 2 times, most recently from 28c0e5f to 3811ad6 Compare September 26, 2024 03:35
@apostasie apostasie marked this pull request as ready for review September 26, 2024 17:42
@apostasie
Copy link
Contributor Author

@AkihiroSuda this is ready - to be merged after #3455

@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Sep 27, 2024
@apostasie
Copy link
Contributor Author

@AkihiroSuda let me rebase this so we get a clean view (and might amend something).
#3435 is more important to review either way (if you do have free cycles).

@apostasie apostasie marked this pull request as draft September 27, 2024 00:21
As we make progress rewriting tests, the new tooling needs to adapt.
In a shell, this is:
- introducing (more) `Requirements`, with a better API
- update documentation
- fix some t.Helper calls
- fix broken stdin implementation
- do cleanup custom namespaces properly
- change hashing function
- disable "private" implying custom data root which is more trouble than is worth
- minor cleanups

Signed-off-by: apostasie <[email protected]>
@apostasie
Copy link
Contributor Author

Hitting 429 with Docker Hub :-(

Otherwise, this is now rebased and ready.

@apostasie apostasie marked this pull request as ready for review September 27, 2024 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants