Test rework, part 3

[apostasie]

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]

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]

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

[apostasie]

@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]

Hitting 429 with Docker Hub :-(

Otherwise, this is now rebased and ready.

[AkihiroSuda]

Needs rebase

[apostasie]

Needs rebase

Will wait for #3483 to land as the CI is currently bust.

[apostasie]

Rebased (w. CI bustage fix)

[AkihiroSuda]

Why change?

[apostasie]

I did hit a case where the message was over 76, which got shortened to a sha256.

Turned out distribution registry rejected it (as an image name), as there is an undocumented limitation rejecting valid image names that are 64-byte hexadecimal strings...

There are also plenty of cases where this fails as an image identifier (with repeat or trailing - or _)

[apostasie]

I will revert this to a readable form later (the sha1 are hurting readability) - it takes a bit more work though to handle the len limitation while keeping readability and avoiding collisions and keeping in line with id grammar.

[apostasie]

Rebased.

CI is green (except the usual windows reg).

[AkihiroSuda]

Do you plan to revert the hashing func?

#3464 (comment)

[apostasie]

Do you plan to revert the hashing func?

#3464 (comment)

Yes - here is the next round version:

nerdctl/pkg/testutil/test/data.go

Lines 124 to 141 in 5c240d7

	func defaultIdentifierHashing(names ...string) string {
	// Notes: this identifier MAY be used for namespaces, image names, etc.
	// So, the rules are stringent on what it can contain.
	replaceWith := []byte("-")
	name := strings.ToLower(strings.Join(names, string(replaceWith)))
	// Ensure we have a unique identifier despite characters replacements (well, as unique as name)
	signature := digest.SHA256.FromString(name).Encoded()[0:8]
	// Make sure we do not use any unsafe characters
	safeName := regexp.MustCompile(`[^a-zA-Z0-9-]+`)
	noRepeat := regexp.MustCompile(fmt.Sprintf(`[%s]{2,}`, replaceWith))
	sn := safeName.ReplaceAll([]byte(name), replaceWith)
	sn = noRepeat.ReplaceAll(sn, replaceWith)
	// Ensure we will never go above 76 characters in length (with signature)
	if len(sn) > 67 {
	sn = sn[0:67]
	}
	return string(sn) + "-" + signature
	}

This seems to bother you - if you prefer I can just backport it here right away - the thing is, this whole thing (rewrite test+tooling) is so huge that it has to be in incremental steps... I appreciate it does introduce a bit of churn, which is always unfortunate...

[apostasie]

Making things simpler - just backported the new version of the hashing method.

Sorry for the confusion about what was meant by "later" (on this PR vs. an upcoming PR)

Pending CI.

[apostasie]

Windows failures are the usual TestRunWithTtyAndDetached aka #3437

[AkihiroSuda]

Thanks