Skip to content

WIP: Windows unit tests - move to junit#39971

Closed
vikramhh wants to merge 1 commit intomoby:masterfrom
vikramhh:unittests_junit_1
Closed

WIP: Windows unit tests - move to junit#39971
vikramhh wants to merge 1 commit intomoby:masterfrom
vikramhh:unittests_junit_1

Conversation

@vikramhh
Copy link

@vikramhh vikramhh commented Sep 22, 2019

Part of work required for #39675 on Windows.
#39638 covers the corresponding work on Linux.

Signed-off-by: Vikram bir Singh [email protected]

- What I did
Added functionality to Unit test execution code to build and use gotestsum.exe for test execution

- How I did it
Modified the Unit test execution code in hack\make.ps1

- How to verify it
Execute unit tests and verify that test output is present in the bundles directory under root.

- Description for the changelog

Build and use gotestsum.exe for unit test execution\

- A picture of a cute animal (not mandatory but encouraged)

@vikramhh vikramhh requested a review from tianon as a code owner September 22, 2019 06:18
@vikramhh vikramhh force-pushed the unittests_junit_1 branch 2 times, most recently from 8e4bb0b to 14f5096 Compare September 22, 2019 21:53
thaJeztah
thaJeztah reviewed Sep 23, 2019
View reviewed changes
thaJeztah
thaJeztah reviewed Sep 23, 2019
View reviewed changes
hack/make.ps1 Outdated
Copy link
Member

@thaJeztah thaJeztah Sep 23, 2019

Choose a reason for hiding this comment

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

Should we build this as part of the Dockerfile, and then copy the binary out?

Copy link
Author

@vikramhh vikramhh Sep 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@thaJeztah - Great suggestion, it would be what we do on Linux. I will put it on my plate - the current approach makes it easier to debug issues with building of gotestsum. Once we have it baked well for all three [unit, integration and integration-cli], I will move this to Dockerfile, if that is OK by you.

@andrewhsu
Copy link
Contributor

andrewhsu commented Sep 23, 2019

Hmm...looks like the windows RS5 node was not allocated properly to even start the tests:

 azwin-2-842900 was marked offline: Connection was broken: java.util.concurrent.TimeoutException: Ping started at 1569247809132 hasn't completed by 1569248049133
 	at hudson.remoting.PingThread.ping(PingThread.java:134)
 	at hudson.remoting.PingThread.run(PingThread.java:90)

I've re-kicked the job.

@andrewhsu andrewhsu mentioned this pull request Sep 23, 2019
Open
8 tasks
@andrewhsu
Copy link
Contributor

andrewhsu commented Sep 24, 2019

You're going to need a section in Jenkinsfile under the windows stages to collect the junit xml files like so:

moby/Jenkinsfile

Lines 401 to 405 in 9b07a3e

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

After you add that, the additional tests will start showing up on the test report: https://ci.docker.com/public/job/moby/view/change-requests/job/PR-39971/test/?width=800&height=600

And under the test results page of the specific build number, e.g.: https://ci.docker.com/public/job/moby/view/change-requests/job/PR-39971/5/testReport/

@vikramhh
Copy link
Author

vikramhh commented Sep 24, 2019

@andrewhsu - thanks, pushed the Jenkinsfile change as well. RS5 for now, will add for RS1 as well after validation.

andrewhsu
andrewhsu reviewed Sep 27, 2019
View reviewed changes
Jenkinsfile
}
post {
always {
junit testResults: 'bundles/junit-report.xml', allowEmptyResults: true
Copy link
Contributor

@andrewhsu andrewhsu Sep 27, 2019

Choose a reason for hiding this comment

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

@vikramhh hmm...i don't see the test results for RS5: https://ci.docker.com/public/job/moby/job/PR-39971/6/testReport/

does windows care about the direction of the slashes in filename paths?

@andrewhsu
Copy link
Contributor

andrewhsu commented Sep 27, 2019

Could you also have Jenkinsfile save the RS5 junit-report.xml to a zip file artifact https://ci.docker.com/public/blue/organizations/jenkins/moby/detail/PR-39971/6/artifacts

It will help with diagnosis and processing of test results.

@thaJeztah
Copy link
Member

thaJeztah commented Sep 27, 2019

I think I'll have to carry this one so that the Jenkinsfile changes are picked up

@thaJeztah thaJeztah mentioned this pull request Sep 27, 2019
Merged
@thaJeztah
Copy link
Member

thaJeztah commented Sep 27, 2019

opened #39998

@thaJeztah
Copy link
Member

thaJeztah commented Oct 7, 2019

I want to try a different approach; perhaps Jenkins doesn't look at where the PR came from (which fork), but only who opened the PR. So instead of forking your branch to my GitHub fork, I want to open a PR from your branch.

GitHub doesn't allow having two PRs open from the same branch though, so I'll close this one, and open a new one from the same branch on your fork.

@thaJeztah thaJeztah closed this Oct 7, 2019
@thaJeztah thaJeztah mentioned this pull request Oct 7, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah left review comments

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

+1 more reviewer

@andrewhsu andrewhsu andrewhsu left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/testing platform/windows status/2-code-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vikramhh @andrewhsu @thaJeztah