-
Notifications
You must be signed in to change notification settings - Fork 273
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
Adding checksum for latest snapshots artifacts #1574
Adding checksum for latest snapshots artifacts #1574
Conversation
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
List <Closure> fileActions = ['createSha512Checksums'] | ||
this.registerLibTester(new UploadMinSnapshotsToS3LibTester( fileActions, 'tests/jenkins/data/opensearch-1.3.0.yml' )) |
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.
Since we are not able to import the lib from vars, I treat this one as a string for comparison @dblock
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.
If there is a way to import it into unit test, please let me know about it.
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 don't understand the problem, what would you like to write here instead of what's in the code?
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.
The original code is:
List <Closure> fileActions = [createSha512Checksums()]
which does not run because it cannot figure out what is createSha512Checksums()
as I cant seems to find a way to import it from /vars/createSha512Checksums.groovy
.
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 see, you could pass another dummy function in here that is a member of the test class, instead of createSha512Checksums
?
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 can, but that kinda defeat the purpose of testing this.
Will try in a different test tho.
Thanks.
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.
@abhinavGupta16 thoughts?
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.
@dblock - A dummy function is a good idea. Although, I don't think a dummy function would do the job either since the passed function in the job is a closure call. The class.name
fetches createSha512Checksums$closurecall_1
and not createSha512Checksums
. Is it possible to create a dummy closure call in this class?
@peterzhuamazon - I think the test would still be competent since we'd only be passing a dummy function in the expected list instead of the list. The rest would still work the same. What do you think?
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 tried declaring a closure but it doesn't get a name. I would just leave this as is.
List <Closure> fileActions = ['createSha512Checksums'] | ||
this.registerLibTester(new UploadMinSnapshotsToS3LibTester( fileActions, 'tests/jenkins/data/opensearch-1.3.0.yml' )) |
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 don't understand the problem, what would you like to write here instead of what's in the code?
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Codecov Report
@@ Coverage Diff @@
## main #1574 +/- ##
============================================
+ Coverage 94.32% 94.35% +0.02%
Complexity 12 12
============================================
Files 148 149 +1
Lines 3154 3170 +16
Branches 8 9 +1
============================================
+ Hits 2975 2991 +16
Misses 172 172
Partials 7 7
Continue to review full report at Codecov.
|
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
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.
Thanks for going beyond the requirements for the task and adding the libtester
for the existing UploadMinSnapshotsToS3LibTester.groovy
library.
* Adding checksum for latest snapshots artifacts Signed-off-by: Peter Zhu <zhujiaxi@amazon.com> * Change to snapshots dir name in path Signed-off-by: Peter Zhu <zhujiaxi@amazon.com> * Update test stack with new location Signed-off-by: Peter Zhu <zhujiaxi@amazon.com> * Simplify the naming in artifacts variables Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: Peter Zhu zhujiaxi@amazon.com
Description
Adding checksum for latest snapshots artifacts
Issues Resolved
#1497
Check List
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.