-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[coverage] fix jest report merging #89537
Conversation
def ARTIFACT_PATTERNS = [ | ||
'target/junit/**/*', | ||
'target/kibana-*', | ||
'target/kibana-coverage/**/*', | ||
'target/kibana-coverage/jest/**/*', |
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.
We don't want to upload content of target/kibana-coverage/functional
since it is 2-10 GB and we archive it here
kibana/vars/kibanaPipeline.groovy
Lines 219 to 223 in 2494b24
if (env.CODE_COVERAGE) { | |
sh 'tar -czf kibana-coverage.tar.gz target/kibana-coverage/**/*' | |
uploadGcsArtifact("kibana-ci-artifacts/jobs/${env.JOB_NAME}/${BUILD_NUMBER}/coverage/${workerName}", 'kibana-coverage.tar.gz') | |
} | |
} |
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.
LGTM - we found FUNCTIONAL and JEST results in kibana-stats :-)
# Need to override COVERAGE_INGESTION_KIBANA_ROOT since json file has original intake worker path | ||
COVERAGE_SUMMARY_FILE=target/kibana-coverage/jest-combined/coverage-summary.json | ||
export COVERAGE_INGESTION_KIBANA_ROOT=/dev/shm/workspace/kibana | ||
node scripts/ingest_coverage.js --verbose --path ${COVERAGE_SUMMARY_FILE} --vcsInfoPath ./VCS_INFO.txt --teamAssignmentsPath $TEAM_ASSIGN_PATH |
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.
Why duplicate instead of loop?
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.
Line #43 is specific for jest, I thought about loop + if but then decided to do it this way
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.
@wayneseymour do you want me to change it with for+if ?
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.
Nah it's all good. I just keep a lookout for duplication.
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
@dmlemeshko, not sure if this is still needed - as the original PR was reverted - and re-merged without changing any of the code coverage information. |
Closing in favour of #90915 |
Summary
This PR fixes code coverage merge for jest: we merge it right after
jest
andjest-integration
tests are finished and upload reports to GCS.Tested: https://kibana-ci.elastic.co/job/elastic+kibana+qa-research/182/