New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Output JUnit XML metadata for CircleCi #12023
Conversation
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
484c8fb
to
55b83ec
Compare
Hey! I know we're getting to this pretty late, but do you mind upstreaming this? I took a pass but got stuck on some of the Flow stuff and thought you might have an easier time with it. Thanks! |
@nhunzaker sure. This week, I have limited access to my laptop unfortunately. I will work on it next week. |
@benbraou no worries at all! I'll check back in about two weeks, but please let me know if you have any questions! |
81ec4b8
to
c7a10e6
Compare
@nhunzaker thanks. I finished rebasing my branch on top of master. No questions for now 😃. I am just curious to see how you are going to validate my changes. Up until now, I was forcing a build failure and then checking that the correct message is displayed in CircleCi summary section (I was also checking the locally generated junit report) |
scripts/circleci/test_entry_point.sh
Outdated
COMMANDS_TO_RUN+=('./scripts/circleci/check_license.sh') | ||
COMMANDS_TO_RUN+=('./scripts/circleci/check_modules.sh') | ||
COMMANDS_TO_RUN+=('./scripts/circleci/test_print_warnings.sh') | ||
COMMANDS_TO_RUN+=('./scripts/circleci/track_stats.sh') | ||
fi | ||
|
||
if [ $((1 % CIRCLE_NODE_TOTAL)) -eq "$CIRCLE_NODE_INDEX" ]; then | ||
COMMANDS_TO_RUN+=('yarn test-prod --maxWorkers=2') | ||
COMMANDS_TO_RUN+=("yarn test-prod --maxWorkers=2 --testResultsProcessor=${JEST_PROCESSOR}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this override the reporter on line 13?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that it does not override but rather duplicate the junit error messages displayed in CircleCi. I am removing this, and keeping the reporter only in line 13.
scripts/circleci/test_entry_point.sh
Outdated
COMMANDS_TO_RUN+=('yarn test-build --maxWorkers=2') | ||
COMMANDS_TO_RUN+=('yarn test-build-prod --maxWorkers=2') | ||
COMMANDS_TO_RUN+=("yarn test-build --maxWorkers=2 --testResultsProcessor=${JEST_PROCESSOR}") | ||
COMMANDS_TO_RUN+=("yarn test-build-prod --maxWorkers=2 --testResultsProcessor=${JEST_PROCESSOR}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question here. Do these need to generate unique reports?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, no need for unique reports. I am removing this, and keeping the reporting only in line 13.
|
||
set -e | ||
|
||
node ./scripts/tasks/junit "$1" "$2" "$3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you leave a comment in this script about what these arguments are?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
Great question. I might have missed something, but these commands need confirmed:
We could just create a failing version of this branch to verify correctness. That'll have to happen for each of these changes. |
@nhunzaker thank you for the review. I have updated the PR with the requested changes. Please let me know if further actions are needed on my side. |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution. |
Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you! |
Fixes #11949
Summary
Currently, when the build fails, there is no way to know what happened other than going through the logs in CircleCI.
The goal of this PR is to output JUnit XML metadata for every build step. This allows CircleCI summary page to display exactly which step has failed.
What this PR does:
jest-junit reporter is used to output JUnit report reports in /junit/jest-results.xml
test_entry_point.sh
is callingyarn test
several times. So, a unit test failure will be duplicated in the build summaryESLint script can either use the default
stylish
formatter orjunit
formatter.junit-report-builder
has been used. Initially, I was writing the JUnit report on my own.Flow result is treated as a single test that has a single test output
Nothing to be done as this script is simply used to extract all messages from `warning()