Windows: Build and use gotestsum for running all tests by thaJeztah · Pull Request #39998 · moby/moby · GitHub

carry of #39971 to make it pick-up the Jenkinsfile changes

I can disable the allowEmptyResults for now, so that it's clear if it failed

Perhaps we should do a dir (e.g.) too see what files are present, and if the location in which it's stored is correct

one failure, which is known to be flaky; #32219

https://ci.docker.com/public/job/moby/job/PR-39998/1/execution/node/209/log/?consoleFull

01:16:22.548      --- FAIL: TestDockerSuite/TestRestartContainerwithRestartPolicy (9.00s)
01:16:22.548          cli.go:29: assertion failed: 
01:16:22.548              Command:  d:\CI\PR-39998\1\binary\docker.exe restart 84a49937203b5fa980e4a6b69ab5af98e8d363e1862ccb9caa49da62015006a5
01:16:22.548              ExitCode: 1
01:16:22.548              Error:    exit status 1
01:16:22.548              Stdout:   
01:16:22.548              Stderr:   Error response from daemon: Cannot restart container 84a49937203b5fa980e4a6b69ab5af98e8d363e1862ccb9caa49da62015006a5: Container 84a49937203b5fa980e4a6b69ab5af98e8d363e1862ccb9caa49da62015006a5 is not running
01:16:22.548              
01:16:22.548              
01:16:22.548              Failures:
01:16:22.548              ExitCode was 1 expected 0
01:16:22.548              Expected no error

It did run the archive junit results step; https://ci.docker.com/public/job/moby/job/PR-39998/1/execution/node/355/log/

But I don't see the test-results for windows;
https://ci.docker.com/public/job/moby/job/PR-39998/1/testReport/

To help with development iteration, you could throw in a temporary git commit to disable tests except for windowsRS5.

moby/Jenkinsfile

Lines 11 to 17 in 5b57f41

    
           booleanParam(name: 'unit_validate', defaultValue: true, description: 'amd64 (x86_64) unit tests and vendor check') 
        
           booleanParam(name: 'amd64', defaultValue: true, description: 'amd64 (x86_64) Build/Test') 
        
           booleanParam(name: 's390x', defaultValue: true, description: 'IBM Z (s390x) Build/Test') 
        
           booleanParam(name: 'ppc64le', defaultValue: true, description: 'PowerPC (ppc64le) Build/Test') 
        
           booleanParam(name: 'windowsRS1', defaultValue: false, description: 'Windows 2016 (RS1) Build/Test') 
        
           booleanParam(name: 'windowsRS5', defaultValue: true, description: 'Windows 2019 (RS5) Build/Test') 
        
           booleanParam(name: 'skip_dco', defaultValue: false, description: 'Skip the DCO check')

Looks like it still doesn't write results in the right location;

https://ci.docker.com/public/job/moby/job/PR-39998/5/execution/node/356/log/

01:01:32.396  Recording test results
01:01:34.755  None of the test reports contained any result

[2019-10-08T17:19:36.991Z] INFO: Invoking unit tests run with C:\gopath/src/gotest.tools/gotestsum\gotestsum.exe --format=standard-quiet --jsonfile=bundles\go-test-report.json --junitfile=bundles\junit-report.xml -- -cover -ldflags -w -tags "autogen daemon" -a "-test.timeout=10m"  .........
...

[2019-10-08T17:23:07.620Z] INFO: make.ps1 ended at 10/08/2019 17:23:07
[2019-10-08T17:23:09.158Z] WARN: junit (Unit test) results(f4eccb3b36:c:\gopath\src\github.com\docker\docker\bundles\junit-report.xml) not found
[2019-10-08T17:23:09.158Z] INFO: Unit tests ended at 10/08/2019 17:23:09. Duration:00:04:10.1162257

@vikramhh I rebased your branch again; I think your local git was not up-to-date with the upstreams.

Should we move this inside the Build-GoTestSum function?

I do not know a compelling argument either way. Why would you choose one over the other?

All other messages (including "Build done ..") are printed as part of the script, so it would make sense to include this as well. If it's move to a script, we could just call Build-GoTestSum and be done

Done

The scripts that we're writing in the Dockerfile now are a bit lengthy, and more complex to maintain (having to keep all the line-continuations (```) into account.

Perhaps we should put these function in a .ps file (easier to write/maintain), and;

Somewhere at the top of the Dockerfile (assuming the functions themself don't change often);

COPY hack/dockerfile/install/gotestsum.installer.ps1 hack/dockerfile/install/gotestsum.installer.ps1

And later on, call those functions

RUN . "hack/dockerfile/install/gotestsum.installer.ps1" Build-GoTestSum

(or however that should be done in PowerShell)

Good suggestion and something I have put on my plate. It will be done in a separate PR

Same here.

Wondering though why we're no longer building the binaries here? (perhaps you can explain why this changed)

Instead of building the test binary and then running it, we are just running the test. Why we need to do it in two different stages?

I think this is to print stderr output if there's no error that happened, correct?

In case of an error ($LASTEXITCODE -ne 0), this will now be printed twice. Think it should be changed to either;

if (($LASTEXITCODE -ne 0) -and ($err -notlike "*warning: no tests to run*")) {
    Throw "Integration tests failed: $err"
} else {
    Write-Host "$err"
}

Or (if Throw exits the script (not sure if if does so?)); perhaps add a comment as well to explain why we're printing $err

if (($LASTEXITCODE -ne 0) -and ($err -notlike "*warning: no tests to run*")) {
    Throw "Integration tests failed: $err"
}

# Write stderr output
Write-Host "$err"

I'm starting to see the junit reports of the test results in jenkins UI now, which is a great step: https://ci.docker.com/public/blue/organizations/jenkins/moby/detail/PR-39998/38/tests

@thaJeztah - the failures on RS5 are known issues. If you are good with the changes, could we get RS1 checks as well.

This PR is valuable as is with RS5 junit test results in jenkins. It is important to expose the failures since the return code is always success.

I'd recommend getting this PR in as soon as possible and having a separate PR for RS1.

The issue we noticed when discussing, is that the naming of the tests for Windows is different than the ones for Linux, therefore the tests show in a different place; for reference, this is how the naming is set for the Linux ones for the junit.xml;

moby/hack/make/.integration-test-helpers

Lines 69 to 90 in e780565

    
           cd "$dir" 
        
           # Create a useful package name based on the tests's $dir. We need to take 
        
           # into account that  "$dir" can be either an absolute (/go/src/github.com/docker/docker/integration/foo) 
        
           # or relative (./integration/foo) path. To account for both, first we strip 
        
           # the absolute path, then remove any leading periods and slashes. 
        
           pkgname="${dir}" 
        
           pkgname="${pkgname#*${GOPATH}/src/${DOCKER_PKG}}" 
        
           pkgname="${pkgname#*.}" 
        
           pkgname="${pkgname#*\/}" 
        
           # Finally, we use periods as separator (instead of slashes) to be more 
        
           # in line with Java package names (which is what junit.xml was designed for) 
        
           pkgname="$(go env GOARCH).${pkgname//\//.}" 
        
           echo "Running $PWD (${pkgname}) flags=${flags}" 
        
           [ -n "$TESTDEBUG" ] && set -x 
        
           # shellcheck disable=SC2086 
        
           test_env gotestsum \ 
        
           	--format=standard-verbose \ 
        
           	--jsonfile="${ABS_DEST}/${pkgname//./-}-go-test-report.json" \ 
        
           	--junitfile="${ABS_DEST}/${pkgname//./-}-junit-report.xml" \ 
        
           	--raw-command \ 
        
           	-- go tool test2json -p "${pkgname}" -t ./test.main ${flags}

The above is true only for integration tests and could be handled by transforming the output of all tests [i.e. the xml files] by applying the same rules to each test phase [unit/integration and integration-cli] once we figure out where each result type should show up. This is not a blocker.

LGTM

@vikramhh could you rebase this PR, because I think the failing test is fixed / skipped on master, so after rebasing, it should be green

Rebased - the two failures are expected because only 1 of the 3 failing tests in #40155 has been disabled. The other two still fail [even though Jenkins does not always mark the checks as failed, something that should be partially fixed by this PR]. The third failure will be fixed by #40193

LGTM

(rebased, to resolve merge-conflict due to Golang bump)

No idea about the powershell, but LGTM! 🐑

remaining failures were tests that don't work on windows, and are now skipped through #40199

rebased to run CI once more, but moving to "merge"

Remaining "failure" is

[2019-12-13T09:38:02.934Z] === Failed
[2019-12-13T09:38:02.934Z] === FAIL: github.com/docker/docker/integration/plugin/volumes  (0.00s)
[2019-12-13T09:38:02.934Z] INFO: Windows Base image is  mcr.microsoft.com/windows/servercore:ltsc2019
[2019-12-13T09:38:02.934Z] ok  	github.com/docker/docker/integration/plugin/volumes	0.325s

Which will be addressed by #40193

1. Dockerfile.Windows modified to build gotestsum.exe 2. Use gotestsum.exe in invoking the execution of: (a) Unit tests (run in containers), (b) Integration tests (run outside containers) (c) Integration-cli (run outside containers) No changes made to other categories of tests (e.g. LCOW). 3. Copy .xml files produced by gotestsum in appropriate paths where Jenkins can ingest them 4. Modify Jenkinsfile to mark results output as being jUnit "type" as well as to archive the .xml test result files as artifacts. Signed-off-by: Vikram bir Singh <vikrambir.singh@docker.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

#40193 was merged, so rebased

All green now. As a follow-up we need to rewrite the test names to match what's used on Linux, so that the Windows tests will appear separate (instead of under github.com/....);

Windows currently produces:

<testsuite tests="22" failures="0" time="28.840000" name="github.com/docker/docker/integration/build">
		<properties>
			<property name="go.version" value="go1.13.4 windows/amd64"></property>
		</properties>
		<testcase classname="github.com/docker/docker/integration/build" name="TestBuildWithSession" time="0.000000">

The same test on Linux (amd64) produces:

	<testsuite tests="32" failures="0" time="171.970000" name="amd64.integration.build">
		<properties>
			<property name="go.version" value="go1.13.4 linux/amd64"></property>
		</properties>
		<testcase classname="amd64.integration.build" name="TestBuildWithSession" time="0.000000">
			<skipped message="=== RUN   TestBuildWithSession&#xA;--- SKIP: TestBuildWithSession (0.00s)&#xA;    build_session_test.go:25: TODO: BuildKit&#xA;"></skipped>

thaJeztah mentioned this pull request Sep 27, 2019

WIP: Windows unit tests - move to junit #39971

Closed

thaJeztah commented Sep 27, 2019

View reviewed changes

thaJeztah force-pushed the carry_39971_unittests_junit_1 branch from 3ebfb8f to af4e179 Compare October 8, 2019 17:07

thaJeztah mentioned this pull request Oct 9, 2019

WIP: Windows unit tests - move to junit #40051

Closed

thaJeztah force-pushed the carry_39971_unittests_junit_1 branch from 0595e1d to 1818270 Compare October 10, 2019 22:54

vikramhh force-pushed the carry_39971_unittests_junit_1 branch from 1818270 to 3cb4def Compare October 11, 2019 04:51

thaJeztah force-pushed the carry_39971_unittests_junit_1 branch from 3cb4def to 8fab97f Compare October 11, 2019 08:21

thaJeztah added area/testing process/cherry-pick status/2-code-review platform/windows labels Oct 11, 2019

vikramhh force-pushed the carry_39971_unittests_junit_1 branch 2 times, most recently from 3b8b468 to 413a18e Compare October 17, 2019 23:35

vikramhh force-pushed the carry_39971_unittests_junit_1 branch 5 times, most recently from 59c3482 to 7408acd Compare October 19, 2019 23:15

thaJeztah changed the title Build and use gotestsum for running unit tests Windows: Build and use gotestsum for running all tests Oct 20, 2019

thaJeztah marked this pull request as ready for review October 20, 2019 14:08

thaJeztah requested a review from tianon as a code owner October 20, 2019 14:08

thaJeztah commented Oct 20, 2019

View reviewed changes

vikramhh force-pushed the carry_39971_unittests_junit_1 branch from 7408acd to eb5e871 Compare October 20, 2019 18:53

vikramhh force-pushed the carry_39971_unittests_junit_1 branch 2 times, most recently from 9c500f5 to 79ebfeb Compare October 28, 2019 23:57

vikramhh force-pushed the carry_39971_unittests_junit_1 branch from a3d9ba5 to 5b8d715 Compare November 2, 2019 18:50

thaJeztah commented Nov 9, 2019

View reviewed changes

vikramhh force-pushed the carry_39971_unittests_junit_1 branch from 5b8d715 to 3c3b9bf Compare November 9, 2019 01:23

vikramhh mentioned this pull request Nov 9, 2019

Volume plugin tests [integration/plugin/volumes] should always be executed - checks for OS Type should happen within the test #40193

Merged

andrewhsu mentioned this pull request Nov 25, 2019

report test results in junit xml format #39675

Open

8 tasks

thaJeztah force-pushed the carry_39971_unittests_junit_1 branch 2 times, most recently from fef5b43 to 10786ba Compare November 25, 2019 14:19

thaJeztah commented Nov 25, 2019

View reviewed changes

cpuguy83 approved these changes Dec 12, 2019

View reviewed changes

thaJeztah force-pushed the carry_39971_unittests_junit_1 branch from 10786ba to 5fc0f6b Compare December 12, 2019 20:00

thaJeztah added status/4-merge and removed status/2-code-review labels Dec 12, 2019

thaJeztah force-pushed the carry_39971_unittests_junit_1 branch from 5fc0f6b to 8d2e1ee Compare December 19, 2019 19:31

thaJeztah merged commit 1a960fe into moby:master Dec 23, 2019

thaJeztah deleted the carry_39971_unittests_junit_1 branch December 23, 2019 12:38

thaJeztah added this to the 20.03.0 milestone Apr 2, 2020

thaJeztah mentioned this pull request Sep 16, 2020

Windows CI: Make sure that CI fails on any error #40599

Merged

thaJeztah removed the process/cherry-pick label Feb 18, 2022

                                   }
                                   post {
                                       always {
+                                          junit testResults: 'bundles/junit-report.xml', allowEmptyResults: true

Windows: Build and use gotestsum for running all tests by thaJeztah · Pull Request #39998 · moby/moby · GitHub

Uh oh!

Windows: Build and use gotestsum for running all tests - #39998

Windows: Build and use gotestsum for running all tests#39998thaJeztah merged 1 commit intomoby:mastermoby/moby:masterfrom thaJeztah:carry_39971_unittests_junit_1thaJeztah/docker:carry_39971_unittests_junit_1Copy head branch name to clipboard

Conversation

thaJeztah commented Sep 27, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Sep 27, 2019

Uh oh!

andrewhsu commented Sep 29, 2019

Uh oh!

thaJeztah commented Oct 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Oct 11, 2019

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewhsu commented Oct 30, 2019

Uh oh!

vikramhh commented Nov 4, 2019

Uh oh!

andrewhsu commented Nov 7, 2019

Uh oh!

thaJeztah commented Nov 7, 2019

Uh oh!

vikramhh commented Nov 8, 2019

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Nov 9, 2019

Uh oh!

vikramhh commented Nov 9, 2019

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

cpuguy83 left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Dec 12, 2019

Uh oh!

thaJeztah commented Dec 12, 2019

Uh oh!

thaJeztah commented Dec 13, 2019

Uh oh!

thaJeztah commented Dec 19, 2019

Uh oh!

thaJeztah commented Dec 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Windows: Build and use gotestsum for running all tests#39998
thaJeztah merged 1 commit into
moby:masterfrom
thaJeztah:carry_39971_unittests_junit_1

thaJeztah commented Oct 9, 2019 •
edited
Loading