-
Notifications
You must be signed in to change notification settings - Fork 67
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 BWC tests for PA #69
Conversation
c08bbb1
to
1d4f7e2
Compare
Codecov Report
@@ Coverage Diff @@
## main #69 +/- ##
============================================
+ Coverage 72.07% 72.11% +0.03%
Complexity 356 356
============================================
Files 44 44
Lines 2510 2510
Branches 160 160
============================================
+ Hits 1809 1810 +1
+ Misses 596 595 -1
Partials 105 105
Continue to review full report at Codecov.
|
9ac68de
to
a5d50ac
Compare
@@ -47,6 +47,12 @@ jobs: | |||
-Dperformance-analyzer-rca.repo="https://github.com/opensearch-project/performance-analyzer-rca.git" \ | |||
-Dperformance-analyzer-rca.branch=main \ | |||
-Dopensearch.version=1.1.0-SNAPSHOT | |||
- name: Assemble PA jar for BWC tests | |||
working-directory: ./tmp/performance-analyzer |
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.
Is this actually supposed to have a preceding '.' in the 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.
PA is checked out in ./tmp/performanceanalyzer
build.gradle
Outdated
@@ -456,6 +466,163 @@ task integTest(type: RestIntegTestTask) { | |||
classpath = sourceSets.test.runtimeClasspath | |||
} | |||
|
|||
testClusters.integTest { |
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.
Is this being used?
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.
Removed this method.
Signed-off-by: Sruti Parthiban <[email protected]>
a5d50ac
to
54d3678
Compare
Signed-off-by: Sruti Parthiban <[email protected]>
break; | ||
case MIXED: | ||
Assert.assertTrue(pluginNames.contains("opensearch-performance-analyzer")); | ||
ensurePAStatus(RestConfig.PA_BASE_URI, true); |
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.
Curious, will there be other tests for features to ensure backwards compatibility?
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 do need to create additional tests to cover other test cases. PA does not create or read any system indices, so all tests will be towards testing the REST API's.
Also using this framework we cannot test the PA RCA component as it needs additional configurations.
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.
Looks like there are merge conflicts, can you please fix those?
Signed-off-by: Sruti Parthiban <[email protected]>
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.
Thank you @sruti1312 for adding these bwc tests! Would love to see further test cases for various upgrade paths.
Signed-off-by: Sruti Parthiban [email protected]
Is your feature request related to a problem? Please provide an existing Issue # , or describe.
#55
Describe the solution you are proposing
Add BWC tests for PA.
Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
Additional context
Add any other context or screenshots about the feature request here.
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.