Skip to content

Generate JUnit Results for E2E Tests on Prow#3542

Merged
chxchx merged 4 commits intoistio:masterfrom
chxchx:junitXML
Feb 22, 2018
Merged

Generate JUnit Results for E2E Tests on Prow#3542
chxchx merged 4 commits intoistio:masterfrom
chxchx:junitXML

Conversation

@chxchx
Copy link
Copy Markdown
Contributor

@chxchx chxchx commented Feb 16, 2018

Also removed the uploadDir() deadcode since we now use bootstrap to update all logs and artifacts automatically when process exits

@chxchx chxchx requested review from a team February 16, 2018 00:31
@istio-merge-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
We suggest the following additional approver: sebastienvas

Assign the PR to them by writing /assign @sebastienvas in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

Gopkg.toml Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With the subvendor repo I think there's now some rule about dependencies have to be updated independent of code changes but I'm not sure if this means the toml file is separate or only the actual upload of the vendor content.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you include junit report in the base image used by prow - like it is done in circleci ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes matt is correct, you need to rebase, rm your vendor dir if it's not the git submodule backed one
do make pull (or make submodule-sync)
then run dep ensure and cd in vendor/ and make a PR for the changes first
https://github.com/istio/istio/wiki/Vendor-FAQ#how-do-i-add--change-a-dependency
I'm glad someone (beside me) will help test/confirm that process

Copy link
Copy Markdown
Member

@ldemailly ldemailly Feb 16, 2018

Choose a reason for hiding this comment

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

oh and costin is correct too, as we don't use junit-report for building istio itself nor its tests, it probably doesn't belong in Gopkg.toml

this being said I wonder how we capture tool chain dependencies/versions ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Junit results harness has moved to makefile so circle ci also benefits from this.

Tool chain dependencies are captured with a version-tracked base docker image. Here is my other PR updating this image per request by Costin https://github.com/istio/test-infra/pull/698/files

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm unclear about what is accomplished by the ">&1". I'd understand "2>&1" but this seems to pipe stdout to itself. It this a no-op or does it have some other side effect? I thought this might be to allow stdout to be both output and captured but that's not what happened for me and instead I had to resort to something like:

make e2e_all > tee >(go-junit-report > junit.xml)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That was my intention to log to console and to stream through pipe. you are right, >&1 really does nothing since the log already prints to stdout by default, which is then dup-ed to whichever file descriptor console/tty is at. What I had was essentially redirecting stdout to stdout... Good catch. I have patched with your fix.

@chxchx chxchx force-pushed the junitXML branch 2 times, most recently from 1b1f7f9 to 72e6cff Compare February 16, 2018 01:36
Copy link
Copy Markdown
Contributor

@costinm costinm left a comment

Choose a reason for hiding this comment

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

Looks good in general, but please address my 2 comments.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you move this to the makefile, so it can be used in circleci too ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Gopkg.toml Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you include junit report in the base image used by prow - like it is done in circleci ?

@yutongz
Copy link
Copy Markdown
Contributor

yutongz commented Feb 16, 2018

/test e2e-simple
/test e2e-bookInfo

We renamed e2e-smoke to e2e-bookInfo and added e2e-simple
ref: #3529

@chxchx
Copy link
Copy Markdown
Contributor Author

chxchx commented Feb 20, 2018

/hold

@istio-testing istio-testing added the do-not-merge/hold Block automatic merging of a PR. label Feb 20, 2018
@istio-merge-robot
Copy link
Copy Markdown

@chxchx PR needs rebase

@istio-merge-robot istio-merge-robot added the needs-rebase Indicates a PR needs to be rebased before being merged label Feb 20, 2018
@istio-merge-robot istio-merge-robot removed the needs-rebase Indicates a PR needs to be rebased before being merged label Feb 20, 2018
istio-merge-robot pushed a commit to istio/test-infra that referenced this pull request Feb 20, 2018
Automatic merge from submit-queue.

Add /opt/go/bin to PATH in Prow Base Image

Required to make go-junit-report binary accessible in prow base image. Helps unblock istio/istio#3542
@chxchx chxchx force-pushed the junitXML branch 4 times, most recently from 11e8a65 to 8f007f4 Compare February 21, 2018 02:17
@ldemailly
Copy link
Copy Markdown
Member

try not force pushing once comments started to come
github tells me I commented but I can't see what I did say

@chxchx chxchx force-pushed the junitXML branch 3 times, most recently from 158c481 to eb303e2 Compare February 21, 2018 23:17
@istio-testing
Copy link
Copy Markdown
Collaborator

@chxchx: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
prow/e2e-smoke.sh 98a5195bb201378b740bbaf1edc2f3db60f24a19 link /test e2e-smoke
prow/istio-pilot-e2e.sh d215af4 link /test istio-pilot-e2e
Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@chxchx chxchx merged commit a5c3c14 into istio:master Feb 22, 2018
@chxchx chxchx removed the do-not-merge/hold Block automatic merging of a PR. label Feb 22, 2018
ChristinaLyu0710-zz pushed a commit to ChristinaLyu0710-zz/istio-flakey-test that referenced this pull request Jun 11, 2019
Automatic merge from submit-queue.

Add /opt/go/bin to PATH in Prow Base Image

Required to make go-junit-report binary accessible in prow base image. Helps unblock istio/istio#3542
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants