Skip to content

Jenkinsfile: create junit.xml for integration tests#39724

Merged
tiborvass merged 4 commits intomoby:masterfrom
thaJeztah:integration_junit
Sep 11, 2019
Merged

Jenkinsfile: create junit.xml for integration tests#39724
tiborvass merged 4 commits intomoby:masterfrom
thaJeztah:integration_junit

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Aug 11, 2019 •
edited
Loading

based on top of / depends on:

Addresses parts of #39675

@thaJeztah thaJeztah requested a review from tianon as a code owner August 11, 2019 18:34
@thaJeztah thaJeztah force-pushed the integration_junit branch 8 times, most recently from ff19427 to 8355b9e Compare August 11, 2019 20:25
@andrewhsu andrewhsu mentioned this pull request Aug 11, 2019
Open
8 tasks
tiborvass
tiborvass reviewed Aug 13, 2019
View reviewed changes
hack/make/.integration-test-helpers Outdated
Copy link
Contributor

@tiborvass tiborvass Aug 13, 2019

Choose a reason for hiding this comment

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

we already have gotestsum installed, let's use that: https://github.com/moby/moby/blob/4e83c90ae8362a8205e3728e8fcb655a713a0d87/hack/dockerfile/install/gotestsum.installer

@thaJeztah thaJeztah force-pushed the integration_junit branch 5 times, most recently from ade2880 to ad4196a Compare August 19, 2019 15:06
@thaJeztah
Copy link
Member Author

thaJeztah commented Aug 19, 2019

One flaky test failed, but otherwise this works; PTAL

I'll squash commits if this looks good

thaJeztah
thaJeztah commented Aug 19, 2019
View reviewed changes
Jenkinsfile Outdated
Copy link
Member Author

@thaJeztah thaJeztah Aug 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

oh; need to remove this one as well; was just there for testing, and no longer works for the "janky" stage, because it's overwritten

@thaJeztah
Copy link
Member Author

thaJeztah commented Aug 19, 2019

Starting to look good

Screenshot 2019-08-19 at 22 58 26

@thaJeztah thaJeztah force-pushed the integration_junit branch 3 times, most recently from 8a44863 to cecca1d Compare August 21, 2019 16:41
@thaJeztah thaJeztah changed the title [WIP] Create junit.xml for integration tests Jenkinsfile: create junit.xml for integration tests Aug 21, 2019
@thaJeztah
Copy link
Member Author

thaJeztah commented Aug 21, 2019

Screenshot 2019-08-21 at 19 40 13

@thaJeztah
Copy link
Member Author

thaJeztah commented Aug 21, 2019

@tiborvass @andrewhsu squashed and cleaned up commits; this should be ready to review / ready to go

After this is merged, we'll need to do another round, and implement it for Windows

@thaJeztah
Copy link
Member Author

thaJeztah commented Sep 11, 2019

Rebased this one, now that #39802 is merged; this should be ready for review/merge

tiborvass
tiborvass reviewed Sep 11, 2019
View reviewed changes
@thaJeztah
integration: use gotestsum to generate junit.xml and go-test-report.json
f3be6b3 
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
integration: test2json: enable timestamps to fix zero-time test durat…
d2e00d6 
…ions

Without these options set, test2json does not include a `Time`
field in the generated JSON;

    {"Action":"run","Test":"TestCgroupNamespacesBuild"}
    {"Action":"output","Test":"TestCgroupNamespacesBuild","Output":"=== RUN   TestCgroupNamespacesBuild\n"}
    {"Action":"output","Test":"TestCgroupNamespacesBuild","Output":"--- PASS: TestCgroupNamespacesBuild (1.70s)\n"}
    ...
    {"Action":"pass","Test":"TestCgroupNamespacesBuild"}

As a result, `gotestsum` was not able to calculate test-duration, and
reported `time="0.000000"` for all tests;

    <testcase classname="amd64.integration.build" name="TestCgroupNamespacesBuild" time="0.000000"></testcase>

With this patch applied:

    {"Time":"2019-08-23T22:42:41.644361357Z","Action":"run","Package":"amd64.integration.build","Test":"TestCgroupNamespacesBuild"}
    {"Time":"2019-08-23T22:42:41.644367647Z","Action":"output","Package":"amd64.integration.build","Test":"TestCgroupNamespacesBuild","Output":"=== RUN   TestCgroupNamespacesBuild\n"}
    {"Time":"2019-08-23T22:42:44.926933252Z","Action":"output","Package":"amd64.integration.build","Test":"TestCgroupNamespacesBuild","Output":"--- PASS: TestCgroupNamespacesBuild (3.28s)\n"}
    ...
    {"Time":"2019-08-23T22:42:44.927003836Z","Action":"pass","Package":"amd64.integration.build","Test":"TestCgroupNamespacesBuild","Elapsed":3.28}

Which now correctly reports the test's duration:

    <testcase classname="amd64.integration.build" name="TestCgroupNamespacesBuild" time="3.280000"></testcase>

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
integration: improve package- and filename for junit.xml
f007b01 
Generate more unique names, based on architecture and test-suite name.

Clean up the path to this integration test to create a useful package name.
"$dir" can be either absolute (/go/src/github.com/docker/docker/integration/foo)
or relative (./integration/foo). To account for both, first we strip the
absolute path, then any leading periods and slashes.

For the package-name, we use periods as separator instead of slashes, to be more
in-line with Java package names (which is what junit.xml was originally designed
for).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Jenkinsfile: add TESTDEBUG
d723643 
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

thaJeztah commented Sep 11, 2019

Disabled TESTDEBUG, and pushed a commit to make a test fail

tiborvass
tiborvass previously approved these changes Sep 11, 2019
View reviewed changes
@tiborvass tiborvass dismissed their stale review September 11, 2019 20:37

lets see how a failure shows up in jenkins

@thaJeztah
Copy link
Member Author

thaJeztah commented Sep 11, 2019

Screenshot 2019-09-11 at 22 46 39

Screenshot 2019-09-11 at 22 47 02

@tiborvass
Copy link
Contributor

tiborvass commented Sep 11, 2019

@thaJeztah ok you can revert the intentional failure and i'll approve.

@thaJeztah
Copy link
Member Author

thaJeztah commented Sep 11, 2019

@tiborvass done 👍

@thaJeztah
Copy link
Member Author

thaJeztah commented Sep 11, 2019

Next steps;

  • implement the same for the Windows stages
  • implement the same for the integration-cli tests (now that we got rid of go-check)

@thaJeztah
Copy link
Member Author

thaJeztah commented Sep 11, 2019

Also (TBD); I read that the junit.xml "standard" expects test durations to be including test setup/teardown. I think currently gotestsum uses the duration of the test itself (which is useful, but having timings including setup/teardown is useful as well)

@tiborvass tiborvass merged commit 234b951 into moby:master Sep 11, 2019
@thaJeztah thaJeztah deleted the integration_junit branch September 11, 2019 22:05
@thaJeztah
Copy link
Member Author

thaJeztah commented Sep 11, 2019

Looks like Windows RS1 nodes are running out of disk? https://ci.docker.com/public/blue/rest/organizations/jenkins/pipelines/moby/branches/PR-39724/runs/52/nodes/50/log/?start=0


[2019-09-11T21:10:43.462Z] powershell.exe : failed to register layer: re-exec error: exit status 1: output: BackupWrite 
[2019-09-11T21:10:43.462Z] \\?\D:\CI-52\CI-d723643dc\daemon\windowsfilter\9e7bef537440980fdd2c1e2a3061dc0f84770c4638b38017e34e5b2e89b09e4a\Files\Windows\INF\cpu.inf: There is not enough space on the disk.
[2019-09-11T21:10:43.462Z] At D:\gopath\src\github.com\docker\docker@tmp\durable-1485d7a6\powershellWrapper.ps1:3 char:1
[2019-09-11T21:10:43.462Z] + & powershell -NoProfile -NonInteractive -ExecutionPolicy Bypass -Comm ...
[2019-09-11T21:10:43.462Z] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[2019-09-11T21:10:43.462Z]     + CategoryInfo          : NotSpecified: (failed to regis...ce on the disk.    :String) [], RemoteException
[2019-09-11T21:10:43.462Z]     + FullyQualifiedErrorId : NativeCommandError
[2019-09-11T21:10:43.462Z]  
[2019-09-11T21:10:43.462Z] 
[2019-09-11T21:10:43.462Z] 
[2019-09-11T21:10:43.462Z] ERROR: Failed 'ERROR: Failed to docker pull mcr.microsoft.com/windows/servercore:ltsc2016 into daemon under test.' at 09/11/2019 21:10:38
[2019-09-11T21:10:43.462Z] At D:\gopath\src\github.com\docker\docker\hack\ci\windows.ps1:708 char:21
[2019-09-11T21:10:43.462Z] + ...             Throw $("ERROR: Failed to docker pull $($env:WINDOWS_BASE ...
[2019-09-11T21:10:43.462Z] +                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[2019-09-11T21:10:43.462Z]

@thaJeztah
Copy link
Member Author

thaJeztah commented Sep 11, 2019

^^ @StefanScherer (also wondering if that can be related to the change to use gopath instead of go; could it be that some cleanup script is not taking that path change into account?

@StefanScherer
Copy link
Contributor

StefanScherer commented Sep 12, 2019

@thaJeztah I had a look at one of the agents.
D:\CI-66 about 22 GB (windowsfilter folder for dut)
D:\docker about 22 GB (windowsfilter folder for docker engine on host)
D:\golang about 550 MB
We have a 64GB disk on D: which is a local attached SSD. This is normally a faster disk than C:

In normal situation the D:\CI-xx folder gets cleaned up. Maybe a dir D:\ during the cleanup phase might help to see if there are orphaned folders?

Do we prune the host docker engine?

I can see this problem 12 times in our logs, starting yesterday.

Maybe the image size of the windowsservercore increased due to the latest Patch Tuesday 🤔

@thaJeztah
Copy link
Member Author

thaJeztah commented Sep 12, 2019

Do we prune the host docker engine?

IIRC the script removes images (except for anything containing servercore) we could try pruning (but should be careful w.r.t. not busting the cache for re-runs on the same PR)

I can see this problem 12 times in our logs, starting yesterday.

Hm.. didn't think of that; wondering if we could check if it increased in size (would be possible if we know the digest of the previous versions)

@thaJeztah
Copy link
Member Author

thaJeztah commented Sep 12, 2019

In normal situation the D:\CI-xx folder gets cleaned up.

I noticed the folder structure is D:\CI-1\CI-<commit hash> first one may be the "run" (haven't checked yet); wondering if we should just change to D:\CI-PR-<number> (not exactly sure what we remove, but there would be no need to keep around files for old commits; especially in case of force-pushing/rebasing

Might of course be that the commit was added to work around cases where we fail to cleanup?

@thaJeztah thaJeztah mentioned this pull request Sep 25, 2019
Merged
This was referenced Oct 3, 2019
Merged
Closed
@thaJeztah thaJeztah added this to the 20.03.0 milestone Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tiborvass tiborvass tiborvass approved these changes

@tianon tianon Awaiting requested review from tianon tianon is a code owner

+2 more reviewers

@andrewhsu andrewhsu andrewhsu approved these changes

@seemethere seemethere seemethere approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@cpuguy83 cpuguy83

Labels

area/testing process/cherry-picked status/2-code-review

Projects

None yet

Milestone

20.10.0

Development

Successfully merging this pull request may close these issues.

7 participants

@thaJeztah @tiborvass @andrewhsu @StefanScherer @seemethere @cpuguy83 @GordonTheTurtle