tests: enabling integration tests on windows

[profnandaa]

Depends on #4994
Started addressing #4485

Starting off with this one as a POC on the approach, before we proceed to complete the rest under /frontend/dockerfile.

Tests covered so far:

• /frontend/dockerfile: testEnvEmptyFormatting
• /frontend/dockerfile: testDockerignoreOverride
• /frontend/dockerfile: caseEmptyDestDir

[tonistiigi]

Atm. all the skipped tests seem to take 40sec to complete (and 46min total).

    --- SKIP: TestIntegration/TestCacheMultiPlatformImportExport/worker=containerd/frontend=builtin (40.72s)
    --- SKIP: TestIntegration/TestCacheMultiPlatformImportExport/worker=containerd/frontend=client (40.29s)
    --- SKIP: TestIntegration/TestAllTargetUnmarshal/worker=containerd/frontend=builtin (40.32s)
    --- SKIP: TestIntegration/TestAllTargetUnmarshal/worker=containerd/frontend=client (40.26s)
    --- SKIP: TestIntegration/TestBaseImagePlatformMismatch/worker=containerd/frontend=client (40.30s)
    --- PASS: TestIntegration/TestEnvEmptyFormatting/worker=containerd/frontend=client (68.83s)
    --- PASS: TestIntegration/TestEnvEmptyFormatting/worker=containerd/frontend=builtin (68.28s)

PTAL why this is so. Maybe something with the registry mirrors and all these tests are doing pulls of images.

[profnandaa]

@tonistiigi -- as for the time being wasted on the skipped tests, I'm wondering if a better approach would be not to enumerate them in the first place. What do you think about this approach? For example:

var unixTests = []func(t *testing.T, sb integration.Sandbox){
	testCmdShell,
	testGlobalArg,
	// ...
}
var windowsTests = []func(t *testing.T, sb integration.Sandbox){}
var allPlatformTests = []func(t *testing.T, sb integration.Sandbox){
	testEnvEmptyFormatting,
	testDockerignoreOverride,
	testEmptyDestDir,
}

var selectedTests = integration.UnixOrWindows(
	[2][]func(t *testing.T, sb integration.Sandbox){unixTests, windowsTests})

var allTests = integration.TestFuncs(append(selectedTests, allPlatformTests...)...)

[tonistiigi]

I'm wondering if a better approach would be not to enumerate them in the first place. What do you think about this approach? For example:

That seems unnecessary as we need to fix the issue with Skip anyway. Eg. I assume that if the empty test has a 40sec overhead, then this same overhead is still applied to all the active tests as well.

[profnandaa]

The immediate killing of the processes was causing the tests to be flaky, with such errors:

=== RUN   TestIntegration/TestEmptyDestDir/worker=containerd/frontend=client
=== PAUSE TestIntegration/TestEmptyDestDir/worker=containerd/frontend=client
=== CONT  TestIntegration/TestEmptyDestDir/worker=containerd/frontend=client
    dockerfile_test.go:543: 
          Error Trace:  D:/a/buildkit/buildkit/frontend/dockerfile/dockerfile_test.go:543
                              D:/a/buildkit/buildkit/util/testutil/integration/run.go:97
                              D:/a/buildkit/buildkit/util/testutil/integration/run.go:209
          Error:        Received unexpected error:
                        failed to compute cache key: hcsshim::ProcessUtilityVMImage \\?\C:\Users\RUNNER~1\AppData\Local\Temp\bktest_containerd1320299573\root\io.containerd.snapshotter.v1.windows\snapshots\4\UtilityVM: Incorrect function.: unknown

Decided to go with 1 sec wait if the process fails to respond to SIGTERM.

[tonistiigi]

Looks good. Some nits.

[profnandaa]

Still we have some flakiness in the pipeline. Can't repro on local though, investigating.

    dockerfile_test.go:434: 
        	Error Trace:	D:/a/buildkit/buildkit/frontend/dockerfile/dockerfile_test.go:434
        	            				D:/a/buildkit/buildkit/util/testutil/integration/run.go:97
        	            				D:/a/buildkit/buildkit/util/testutil/integration/run.go:209
        	Error:      	Received unexpected error:
        	            	failed to compute cache key: hcsshim::ProcessUtilityVMImage \\?\C:\Users\RUNNER~1\AppData\Local\Temp\bktest_containerd1251838708\root\io.containerd.snapshotter.v1.windows\snapshots\3\UtilityVM: Incorrect function.: unknown
        	            	github.com/moby/buildkit/util/stack.Enable
        	            		D:/a/buildkit/buildkit/util/stack/stack.go:78
        	            	github.com/moby/buildkit/util/grpcerrors.FromGRPC
        	            		D:/a/buildkit/buildkit/util/grpcerrors/grpcerrors.go:198
        	            	github.com/moby/buildkit/util/grpcerrors.UnaryClientInterceptor
        	            		D:/a/buildkit/buildkit/util/grpcerrors/intercept.go:41
        	            	google.golang.org/grpc.(*ClientConn).Invoke
        	            		D:/a/buildkit/buildkit/vendor/google.golang.org/grpc/call.go:35
        	            	github.com/moby/buildkit/api/services/control.(*controlClient).Solve
        	            		D:/a/buildkit/buildkit/api/services/control/control.pb.go:2252
        	            	github.com/moby/buildkit/client.(*Client).solve.func2
        	            		D:/a/buildkit/buildkit/client/solve.go:270
        	            	golang.org/x/sync/errgroup.(*Group).Go.func1
        	            		D:/a/buildkit/buildkit/vendor/golang.org/x/sync/errgroup/errgroup.go:78
        	            	runtime.goexit
        	            		C:/hostedtoolcache/windows/go/1.22.5/x64/src/runtime/asm_amd64.s:1695
        	            	failed to solve
        	            	github.com/moby/buildkit/client.(*Client).solve.func2
        	            		D:/a/buildkit/buildkit/client/solve.go:286
        	            	golang.org/x/sync/errgroup.(*Group).Go.func1
        	            		D:/a/buildkit/buildkit/vendor/golang.org/x/sync/errgroup/errgroup.go:78
        	            	runtime.goexit
        	            		C:/hostedtoolcache/windows/go/1.22.5/x64/src/runtime/asm_amd64.s:1695
        	Test:       	TestIntegration/TestEnvEmptyFormatting/worker=containerd/frontend=client

[profnandaa]

I could finally repro it on my local too (just needed to run the whole test suite with gotestsum [1] instead of running individual tests as I was doing). There seems to be some certain race condition. When I removed --parallel option for gotestsum, the tests are now running okay.

--- a/.github/workflows/test-os.yml
+++ b/.github/workflows/test-os.yml
@@ -77,7 +77,7 @@ jobs:
     needs:
       - build
     env:
-      TESTFLAGS: "-v --parallel=6 --timeout=30m"
+      TESTFLAGS: "-v --timeout=60m"

[1] gotestsum

gotestsum     --jsonfile="./bin/testreports/go-test-report-test-windows-amd64-frontenddockerfile--containerd.json"     --junitfile="./bin/testreports/junit-report-test-windows-amd64-frontenddockerfile--containerd.xml"     --packages="./frontend/dockerfile"     --       "-mod=vendor"       "-coverprofile" "./bin/testreports/coverage-test-windows-amd64-frontenddockerfile--containerd.txt"       "-covermode" "atomic" -v --timeout=60m --run=TestIntegration/.*/worker=containerd

Let me revisit the running in parallel option later. Noting it on #4485

[profnandaa]

Seems I was wrong on my analysis still. The runner is generally flaky. Let me investigate further...