use designated domains in tests (RFC2606)

[thaJeztah]

• [x] depends on https://github.com/docker/docker-credential-helpers/pull/275

Update domains used in tests to used domains that are designated for this purpose as described in RFC2606, section 3

[codecov-commenter]

Codecov Report

Patch coverage has no change and project coverage change: -4.33 :warning:

Comparison is base (7f48455) 52.51% compared to head (834fe12) 48.18%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #274      +/-   ##
==========================================
- Coverage   52.51%   48.18%   -4.33%     
==========================================
  Files           9        8       -1     
  Lines         676      579      -97     
==========================================
- Hits          355      279      -76     
+ Misses        274      261      -13     
+ Partials       47       39       -8     

see 1 file with indirect coverage changes

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[thaJeztah]

oh! found some more; hold on a minute

hm.. also interested why CI fails; need to check

[crazy-max]

oh! found some more; hold on a minute

hm.. also interested why CI fails; need to check

Hum yes it seems path query is not taken into account anymore https://github.com/docker/docker-credential-helpers/actions/runs/5098464489/jobs/9165620051?pr=274#step:7:66

=== RUN   TestWinCredHelperStoreRetrieve
    wincred_windows_test.go:230: Error: expected username user-7, got username user-6 for URL: https://foobar.example.com:2376/some/other/path?foo=bar
    wincred_windows_test.go:233: Error: expected secret secret-7, got secret secret-6 for URL: https://foobar.example.com:2376/some/other/path?foo=bar

[crazy-max]

It should exact match properly though https://github.com/docker/docker-credential-helpers/blob/11e6d3772ce263a74a2d150e858f23b975c4e374/wincred/wincred_windows.go#L110

[thaJeztah]

Yeah; want to give it a look indeed.

Also change these tests into subtests probably

[thaJeztah]

(only addressed the "found some more" - not looked at the failures yet)

[thaJeztah]

I rebased this PR on top of the other PR that adds subtests;

=== RUN   TestWinCredHelperStoreRetrieve/https://foobar.example.com:2376/some/other/path?foo=bar
    wincred_windows_test.go:274: Error: expected username user-7, got username user-6 for URL: https://foobar.example.com:2376/some/other/path?foo=bar
    wincred_windows_test.go:277: Error: expected secret secret-7, got secret secret-6 for URL: https://foobar.example.com:2376/some/other/path?foo=bar

The test that's failing uses the same domain and path as the test before it, but differs in query parameters https://github.com/docker/docker-credential-helpers/blob/f09e79d7419a4122052bc8fc24f2a36aaecc2495/wincred/wincred_windows_test.go#L195-L196

And the test is using a test-table, which adds the credentials as part of the table for each test, but does not delete the credentials after testing; https://github.com/docker/docker-credential-helpers/blob/f09e79d7419a4122052bc8fc24f2a36aaecc2495/wincred/wincred_windows_test.go#L213-L235

I wonder if the issue is if

• wincred.Add() possibly update secrets if it already found an existing one for the given host / URL?
• and if it considers https://foobar.docker.io:2376/some/other/path to be the same as https://foobar.docker.io:2376/some/other/path?foo=bar
• ^^ and because of that, doesn't update the credentials

The .Add() is designed to effectively be an "Upsert" (create if not exists, otherwise update). For the macOS keychain helper, I noticed that there's an explicit Delete() at the start; https://github.com/docker/docker-credential-helpers/blob/f09e79d7419a4122052bc8fc24f2a36aaecc2495/osxkeychain/osxkeychain_darwin.go#L35-L37

The wincred.Add() inmplementation does not have that; https://github.com/docker/docker-credential-helpers/blob/f09e79d7419a4122052bc8fc24f2a36aaecc2495/wincred/wincred_windows.go#L16-L26

Some of the other tests call a Delete() as part of the test, e.g. TestWinCredHelperRetrieveAliases; https://github.com/docker/docker-credential-helpers/blob/f09e79d7419a4122052bc8fc24f2a36aaecc2495/wincred/wincred_windows_test.go#L128

So, I suspect what happens is that;

• the https://foobar.docker.io:2376/some/other/path test runs (succesfully), which adds credentials for that URL
• the https://foobar.docker.io:2376/some/other/path?foo=bar test runs; does an Add(), but the credentials store already has an entry for https://foobar.docker.io:2376/some/other/path, and doesn't actually update?
• question is: why does it fail NOW ? All I changed in this PR is the domain (https://foobar.docker.io:2376 -> https://foobar.example.com:2376), which should not matter for this test.