Skip to content

ci: test-unit job matrix for win/macos/ubuntu - #2206

Merged
crazy-max merged 3 commits into
docker:masterfrom
crazy-max:test-os
Jan 26, 2024
Merged

ci: test-unit job matrix for win/macos/ubuntu#2206
crazy-max merged 3 commits into
docker:masterfrom
crazy-max:test-os

Conversation

@crazy-max

Copy link
Copy Markdown
Member

follow-up #2205 (comment)

@crazy-max

Copy link
Copy Markdown
MemberAuthor

Some tests look to fail on windows runner: https://github.com/docker/buildx/actions/runs/7640538748/job/20815891223#step:5:419

@crazy-maxcrazy-max changed the title ci: run unit tests on windowsci: test-os job for unit testsJan 25, 2024
@crazy-max
crazy-maxforce-pushed the test-os branch 4 times, most recently from 61852c8 to 173a44dCompareJanuary 25, 2024 08:45
@crazy-maxcrazy-max changed the title ci: test-os job for unit testsci: test-unit job matrix for win/macos/ubuntuJan 25, 2024
@crazy-max
crazy-maxforce-pushed the test-os branch 3 times, most recently from 5137a6c to 947498aCompareJanuary 25, 2024 09:20
@crazy-max

crazy-max commented Jan 25, 2024

Copy link
Copy Markdown
MemberAuthor

This one is interesting when running tests on macOS runner when temp dir is the default one: https://github.com/docker/buildx/actions/runs/7652363539/job/20851977017?pr=2206#step:6:863

=== Failed
=== FAIL: controller/pb TestResolvePaths/contextpath (0.00s)
path_test.go:244: expected pb.BuildOptions{ContextPath:"/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/testresolvepaths759994391/test", DockerfileName:"", PrintFunc:(*pb.PrintFunc)(nil), NamedContexts:map[string]string(nil), Allow:[]string(nil), Attests:[]*pb.Attest(nil), BuildArgs:map[string]string(nil), CacheFrom:[]*pb.CacheOptionsEntry(nil), CacheTo:[]*pb.CacheOptionsEntry(nil), CgroupParent:"", Exports:[]*pb.ExportEntry(nil), ExtraHosts:[]string(nil), Labels:map[string]string(nil), NetworkMode:"", NoCacheFilter:[]string(nil), Platforms:[]string(nil), Secrets:[]*pb.Secret(nil), ShmSize:0, SSH:[]*pb.SSH(nil), Tags:[]string(nil), Target:"", Ulimits:(*pb.UlimitOpt)(nil), Builder:"", NoCache:false, Pull:false, ExportPush:false, ExportLoad:false, SourcePolicy:(*moby_buildkit_v1_sourcepolicy.Policy)(nil), Ref:"", GroupRef:"", Annotations:[]string(nil), XXX_NoUnkeyedLiteral:struct {}{}, XXX_unrecognized:[]uint8(nil), XXX_sizecache:0}, got pb.BuildOptions{ContextPath:"/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/testresolvepaths759994391/test", DockerfileName:"", PrintFunc:(*pb.PrintFunc)(nil), NamedContexts:map[string]string(nil), Allow:[]string(nil), Attests:[]*pb.Attest(nil), BuildArgs:map[string]string(nil), CacheFrom:[]*pb.CacheOptionsEntry(nil), CacheTo:[]*pb.CacheOptionsEntry(nil), CgroupParent:"", Exports:[]*pb.ExportEntry(nil), ExtraHosts:[]string(nil), Labels:map[string]string(nil), NetworkMode:"", NoCacheFilter:[]string(nil), Platforms:[]string(nil), Secrets:[]*pb.Secret(nil), ShmSize:0, SSH:[]*pb.SSH(nil), Tags:[]string(nil), Target:"", Ulimits:(*pb.UlimitOpt)(nil), Builder:"", NoCache:false, Pull:false, ExportPush:false, ExportLoad:false, SourcePolicy:(*moby_buildkit_v1_sourcepolicy.Policy)(nil), Ref:"", GroupRef:"", Annotations:[]string(nil), XXX_NoUnkeyedLiteral:struct {}{}, XXX_unrecognized:[]uint8(nil), XXX_sizecache:0}
--- FAIL: TestResolvePaths/contextpath (0.00s)

Note the diff with ContextPath:

/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/testresolvepaths759994391/test
vs
/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/testresolvepaths759994391/test

cc @tonistiigi@jedevc

@crazy-max
crazy-maxforce-pushed the test-os branch 3 times, most recently from f3c264e to b04b304CompareJanuary 25, 2024 09:39
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
Comment threadbuild/git_windows.go
Comment on lines +5 to +26
// getLongPathName converts Windows short pathnames to full pathnames.
// For example C:\Users\ADMIN~1 --> C:\Users\Administrator.
func getLongPathName(path string) (string, error) {
// See https://groups.google.com/forum/#!topic/golang-dev/1tufzkruoTg
p, err := windows.UTF16FromString(path)
if err != nil {
return "", err
}
b := p // GetLongPathName says we can reuse buffer
n, err := windows.GetLongPathName(&p[0], &b[0], uint32(len(b)))
if err != nil {
return "", err
}
if n > uint32(len(b)) {
b = make([]uint16, n)
_, err = windows.GetLongPathName(&p[0], &b[0], uint32(len(b)))
if err != nil {
return "", err
}
}
return windows.UTF16ToString(b), nil
}

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Comment on lines +11 to +14
expected := "C:\\Users\\foobar"
if isGitBash() {
expected = "C:/Users/foobar"
}

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

As we are using Git bash on ci to run tests, the behavior is not the same.

crazy-maxand others added 2 commits January 25, 2024 15:06
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
@crazy-max
crazy-max marked this pull request as ready for review January 25, 2024 14:18
@crazy-max

Copy link
Copy Markdown
MemberAuthor

Second commit is a bug on Windows. Let me know if you want this in a dedicated PR. Should be backported as well.

Comment threadbuild/git_windows.go
@@ -0,0 +1,26 @@
package build

@crazy-maxcrazy-maxJan 25, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Not sure if we should put this in this package or util one. Maybe on buildkit as it might be useful for Windows support (cc @TBBle@gabriel-samfira)

Comment on lines -74 to -76
include:
- pkg: ./...
skip-integration-tests: 1

@crazy-maxcrazy-maxJan 25, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@jedevc I removed this one as it's already handled in test-unit job through ubuntu-latest otherwise we have dup testing. Let me know if that sounds good.

uses: codecov/codecov-action@v3
with:
directory: ./bin/testreports
flags: integration

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Added some flags for codecov so we have a clear distinction between unit and integration tests

@crazy-max

Copy link
Copy Markdown
MemberAuthor

Needs follow-up for client-side integration tests on windows/macos

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@crazy-max@tonistiigi