Skip to content
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

Add library to create test report manifest #270

Merged
merged 4 commits into from
Aug 18, 2023

Conversation

zelinh
Copy link
Member

@zelinh zelinh commented Aug 11, 2023

Description

Add groovy library to trigger report workflow to create test report manifest. Upload the generated test-report.yml to the S3 test results bucket.

Issues Resolved

Part of opensearch-project/opensearch-build#1274

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Merging #270 (1937e74) into main (9539179) will increase coverage by 0.55%.
Report is 4 commits behind head on main.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main     #270      +/-   ##
============================================
+ Coverage     86.63%   87.19%   +0.55%     
- Complexity       27       28       +1     
============================================
  Files            77       78       +1     
  Lines           202      203       +1     
  Branches         11       11              
============================================
+ Hits            175      177       +2     
  Misses           19       19              
+ Partials          8        7       -1     
Files Changed Coverage Δ
...ns/jobs/CreateUploadTestReportManifest_Jenkinsfile 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

"--test-run-id ${testID}",
"--test-type ${testType}",
"--base-path ${basePath}",
"--component ${component}",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since the component is an optional parameter, you can have a null check like here and then change it to
isNullOrEmpty(args.componentName.toString()) ? "" : "--component ${args.componentName}",

Copy link
Member Author

Choose a reason for hiding this comment

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

Good suggestion. Updating here. Thanks!

@zelinh
Copy link
Member Author

zelinh commented Aug 14, 2023

Just had a new commit to accommodate your suggestions. Please help take a look again. Thanks! @gaiksaya @rishabh6788

super.setUp()
super.testPipeline("tests/jenkins/jobs/CreateUploadTestReportManifest_Jenkinsfile")
assertThat(getShellCommands('sh', 'report.sh'), hasItems('./report.sh tests/data/opensearch-1.3.0-test.yml --artifact-paths opensearch=https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/1.3.0/c3ff7a232d25403fa8cc14c97799c323/linux/x64/tar --test-run-id 1234 --test-type integ-test --base-path null/null/1.3.0/c3ff7a232d25403fa8cc14c97799c323/linux/x64/tar '))
assertThat(getShellCommands('sh', 'report.sh'), hasItems('./report.sh tests/data/opensearch-1.3.0-test.yml --artifact-paths opensearch=https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/1.3.0/c3ff7a232d25403fa8cc14c97799c323/linux/x64/tar opensearch-dashboards=https://ci.opensearch.org/ci/dbc/distribution-build-opensearch-dashboards/1.3.0/25b38c278cdd45efa583765d8ba76346/linux/x64/tar --test-run-id 1234 --test-type integ-test --base-path null/null/1.3.0/c3ff7a232d25403fa8cc14c97799c323/linux/x64/tar '))
Copy link
Member

Choose a reason for hiding this comment

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

This should have opensearch-dashboards test manifest. Also base path is going null here:
--base-path null/null/1.3.0/c3ff7a232d25403fa8cc14c97799c323/linux/x64/tar

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. Updated here with a new OSD test manifest for 1.3.0.

Comment on lines 33 to 39
binding.setVariable('env', ['JOB_NAME': 'dummy_integ_test'])
binding.setVariable('ARTIFACT_BUCKET_NAME', 'DUMMY_BUCKET_NAME')
binding.setVariable('AWS_ACCOUNT_PUBLIC', 'DUMMY_AWS_ACCOUNT_PUBLIC')
binding.setVariable('ARTIFACT_BUCKET_NAME', 'DUMMY_ARTIFACT_BUCKET_NAME')
binding.setVariable('PUBLIC_ARTIFACT_URL', 'DUMMY_PUBLIC_ARTIFACT_URL')
binding.setVariable('env', ['BUILD_NUMBER': '487'])
binding.setVariable('STAGE_NAME', 'DUMMY_STAGE_NAME')
Copy link
Member

Choose a reason for hiding this comment

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

None of these variables are used in this library. Can you check and remove the unsed ones?
Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah you are right! and also explains those null in the base-path. Updated in the latest commit.

Signed-off-by: Zelin Hao <[email protected]>
@zelinh zelinh force-pushed the jenkins_report branch 6 times, most recently from 889a941 to e45cfd9 Compare August 16, 2023 01:09
def call(Map args = [:]) {
lib = library(identifier: 'jenkins@main', retriever: legacySCM(scm))

if (!parameterCheck(args.testManifest, args.buildManifest, args.testRunID, args.testType)) return
Copy link
Member

Choose a reason for hiding this comment

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

What is this returning?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not returning anything. It's returned from this library and skips the rest of code below.

def call(Map args = [:]) {
lib = library(identifier: 'jenkins@main', retriever: legacySCM(scm))

if (!parameterCheck(args.testManifest, args.buildManifest, args.testRunID, args.testType)) return
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (!parameterCheck(args.testManifest, args.buildManifest, args.testRunID, args.testType)) return
if (!parameterCheck(args.testManifest, args.buildManifest, args.testRunID, args.testType)) return null

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated as suggested.

Signed-off-by: Zelin Hao <[email protected]>
Copy link
Member

@gaiksaya gaiksaya left a comment

Choose a reason for hiding this comment

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

Please bump the version to next minor.

This is done.
Thanks!

@gaiksaya gaiksaya merged commit b5c7537 into opensearch-project:main Aug 18, 2023
7 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 18, 2023
Signed-off-by: Zelin Hao <[email protected]>
(cherry picked from commit b5c7537)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants