tests: enable integration test run on windows as baseline #4479

profnandaa · 2023-12-12T05:34:16Z

The plan is to skip all the failing tests on windows so as to have the CI running on WS2022; and then we will go fixing each tests and enabling them in separate PRs -- see tracking issue here #4485

This will be a helpful approach as we bring parity between windows and unix.

I think skipping failing tests on Windows is ok to get a general view of which ones currently do not work. As we move forward, we can move them one by one in a platform specific _test.go file if they don't make sense at all on Windows (like testShmSize) and enable the ones that should absolutely work on Windows such as: https://github.com/moby/buildkit/pull/4479/files#diff-93731187c2a292b7e92b852225675f88b41b7634dafd66cfeab50697bd806ddbR3053
If we ever manage to enable LCOW support, then we can enable linux specific tests on Windows as well, but until then we need to focus on WCOW. -- @gabriel-samfira

util/testutil/workers/oci_windows.go

.github/workflows/test-os.yml

gabriel-samfira · 2023-12-12T08:32:38Z

I think skipping failing tests on Windows is ok to get a general view of which ones currently do not work. As we move forward, we can move them one by one in a platform specific _test.go file if they don't make sense at all on Windows (like testShmSize) and enable the ones that should absolutely work on Windows such as:

https://github.com/moby/buildkit/pull/4479/files#diff-93731187c2a292b7e92b852225675f88b41b7634dafd66cfeab50697bd806ddbR3053

If we ever manage to enable LCOW support, then we can enable linux specific tests on Windows as well, but until then we need to focus on WCOW.

util/testutil/integration/run.go

jedevc · 2023-12-12T10:53:48Z

client/client_test.go

-			"dns": bridgeDNSNetwork,
-		}),
-	)
+	if runtime.GOOS != "windows" {


Each individual test in testBridgeNetworkingDNSNoRootless should instead declare that it cannot be run in windows, right?

I want to avoid as many explicit runtime.GOOS != "windows" checks as possible, they're a little inflexible, and complicate reading the code.

For some reason, even with the line to skip at the beginning of the function, some set up was still being done; so I'd to guard this. Any tips will be appreciated.

Here's the trace:

Error Trace: C:/buildkit/util/testutil/integration/run.go:198 Error: Received unexpected error: process exited: C:\Users\Administrator\go\bin\buildkitd.exe --containerd-worker-gc=false --containerd-worker=true --containerd-worker-addr npipe:////./pipe/containerd-bktest_containerd2165329572 --containerd-worker-labels=org.mobyproject.buildkit.worker.sandbox=true --config=C:\Users\ADMINI~1\AppData\Local\Temp\2\bktest_config1965241481\buildkitd.toml --root C:\Users\ADMINI~1\AppData\Local\Temp\2\bktest_buildkitd2000050298 --addr npipe:////./pipe/buildkitd-bktest_buildkitd2000050298 --debug github.com/moby/buildkit/util/testutil/integration.WaitSocket C:/buildkit/util/testutil/integration/util.go:111 github.com/moby/buildkit/util/testutil/workers.runBuildkitd C:/buildkit/util/testutil/workers/util.go:93 github.com/moby/buildkit/util/testutil/workers.(*Containerd).New C:/buildkit/util/testutil/workers/containerd.go:228 github.com/moby/buildkit/util/testutil/integration.newSandbox C:/buildkit/util/testutil/integration/sandbox.go:100 github.com/moby/buildkit/util/testutil/integration.Run.func3.1 C:/buildkit/util/testutil/integration/run.go:197 testing.tRunner C:/Program Files/Go/src/testing/testing.go:1595 runtime.goexit C:/Program Files/Go/src/runtime/asm_amd64.s:1650 creating worker github.com/moby/buildkit/util/testutil/integration.newSandbox C:/buildkit/util/testutil/integration/sandbox.go:102 github.com/moby/buildkit/util/testutil/integration.Run.func3.1 C:/buildkit/util/testutil/integration/run.go:197 testing.tRunner C:/Program Files/Go/src/testing/testing.go:1595 runtime.goexit C:/Program Files/Go/src/runtime/asm_amd64.s:1650 Test: TestIntegration/TestBridgeNetworkingDNSNoRootless/worker=containerd/netmode=dns --- FAIL: TestIntegration/TestBridgeNetworkingDNSNoRootless/worker=containerd/netmode=dns (20.22s) ```

.github/workflows/test-os.yml

The plan is to enable to skip all the failing tests on windows so as to have the CI running on WS2022; and then we will go fixing each tests and enabling them in separate PRs. This is the baseline. This will be a helpful approach as we bring parity between windows and unix. Signed-off-by: Anthony Nandaa <profnandaa@gmail.com>

tonistiigi · 2023-12-14T00:02:08Z

.github/workflows/test-os.yml

@@ -20,7 +20,7 @@ on:

 env:
  GO_VERSION: "1.21"
-  TESTFLAGS: "-v --parallel=6 --timeout=30m"
+  TESTFLAGS: "-v --parallel=6 --timeout=60m"


not for this PR but if this gets too long we need to split this suite to multiple parallel workers

Sure, for now we just have those 3.

tonistiigi · 2023-12-14T00:05:53Z

.github/workflows/test-os.yml

+          $version = [regex]::Match($goModFileContent, $pattern).Value
+          mkdir -p .\bin\installs -f
+          cd .\bin\installs
+          git clone https://github.com/containerd/containerd.git


Eventually, this would need to be done with the Dockerfile. Either directly on windows or cross-compiled on linux. Eg. freebsd task in this file does the cross-compile.

Good call out. Eventually, we will need to explore buildkitd running in the container (WCOW).

Yes and same for distribution binaries, can look at this in follow-up 👍

crazy-max

LGTM to start with thanks

As follow-up we should cross comp container/distribution binaries to avoid drifting like freebsd one and most of this logic should be moved to https://github.com/moby/buildkit/blob/master/.github/workflows/.test.yml where we are self-contained and run integration tests through a WCOW container. As I see it we would have a common .test.yml that would either call a .test-linux.yml, .test-windows.yml reusable workflow depending on a new "os" input. We could then also have a .test-freebsd.yml so we can remove the test-os.yml workflow.

crazy-max · 2023-12-14T08:11:42Z

.github/workflows/test-os.yml

+          $version = [regex]::Match($goModFileContent, $pattern).Value
+          mkdir -p .\bin\installs -f
+          cd .\bin\installs
+          git clone https://github.com/containerd/containerd.git


Yes and same for distribution binaries, can look at this in follow-up 👍

gabriel-samfira

LGTM. The tests directly under ./frontend might pass. Could you try them out locally and maybe enable them if they do? Would be a shame not to run them if they work.

frontend/frontend_test.go

profnandaa · 2023-12-14T09:45:11Z

@crazy-max -- thanks, taking note of that. Adding it as a task on the main tracking issue here #4485

profnandaa force-pushed the windows/int-test-run branch 2 times, most recently from 13cf1c3 to 54282a9 Compare December 12, 2023 05:38

profnandaa commented Dec 12, 2023

View reviewed changes

util/testutil/workers/oci_windows.go Outdated Show resolved Hide resolved

gabriel-samfira reviewed Dec 12, 2023

View reviewed changes

.github/workflows/test-os.yml Outdated Show resolved Hide resolved

jedevc reviewed Dec 12, 2023

View reviewed changes

profnandaa changed the title ~~tests: enable integration test run on windows~~ tests: enable integration test run on windows as baseline Dec 13, 2023

profnandaa force-pushed the windows/int-test-run branch 3 times, most recently from 65cee0f to 8f4aad6 Compare December 13, 2023 07:29

profnandaa marked this pull request as ready for review December 13, 2023 07:30

profnandaa commented Dec 13, 2023

View reviewed changes

.github/workflows/test-os.yml Show resolved Hide resolved

profnandaa commented Dec 13, 2023

View reviewed changes

.github/workflows/test-os.yml Show resolved Hide resolved

profnandaa force-pushed the windows/int-test-run branch 6 times, most recently from 1ef27eb to fe257a5 Compare December 13, 2023 21:33

profnandaa force-pushed the windows/int-test-run branch from fe257a5 to 4b7c173 Compare December 13, 2023 21:39

tonistiigi reviewed Dec 14, 2023

View reviewed changes

tonistiigi approved these changes Dec 14, 2023

View reviewed changes

crazy-max approved these changes Dec 14, 2023

View reviewed changes

gabriel-samfira reviewed Dec 14, 2023

View reviewed changes

frontend/frontend_test.go Show resolved Hide resolved

frontend/frontend_test.go Show resolved Hide resolved

profnandaa mentioned this pull request Dec 14, 2023

WCOW/tests: revisit skipped integration tests to run on Windows where applicable #4485

Open

gabriel-samfira merged commit 8c4da32 into moby:master Dec 14, 2023
60 checks passed

profnandaa deleted the windows/int-test-run branch December 18, 2023 05:07

crazy-max added the area/testing label Dec 18, 2023

crazy-max added the area/windows label Dec 18, 2023

crazy-max mentioned this pull request Feb 15, 2024

ci(test-os): build binaries from dockerfile for windows #4652

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: enable integration test run on windows as baseline #4479

tests: enable integration test run on windows as baseline #4479

profnandaa commented Dec 12, 2023 •

edited

Loading

gabriel-samfira commented Dec 12, 2023

jedevc Dec 12, 2023

profnandaa Dec 13, 2023

tonistiigi Dec 14, 2023

profnandaa Dec 14, 2023

tonistiigi Dec 14, 2023

profnandaa Dec 14, 2023 •

edited

Loading

crazy-max Dec 14, 2023

crazy-max left a comment

crazy-max Dec 14, 2023

gabriel-samfira left a comment

profnandaa commented Dec 14, 2023

tests: enable integration test run on windows as baseline #4479

tests: enable integration test run on windows as baseline #4479

Conversation

profnandaa commented Dec 12, 2023 • edited Loading

gabriel-samfira commented Dec 12, 2023

jedevc Dec 12, 2023

Choose a reason for hiding this comment

profnandaa Dec 13, 2023

Choose a reason for hiding this comment

tonistiigi Dec 14, 2023

Choose a reason for hiding this comment

profnandaa Dec 14, 2023

Choose a reason for hiding this comment

tonistiigi Dec 14, 2023

Choose a reason for hiding this comment

profnandaa Dec 14, 2023 • edited Loading

Choose a reason for hiding this comment

crazy-max Dec 14, 2023

Choose a reason for hiding this comment

crazy-max left a comment

Choose a reason for hiding this comment

crazy-max Dec 14, 2023

Choose a reason for hiding this comment

gabriel-samfira left a comment

Choose a reason for hiding this comment

profnandaa commented Dec 14, 2023

profnandaa commented Dec 12, 2023 •

edited

Loading

profnandaa Dec 14, 2023 •

edited

Loading