BWC test workflow #1603
Merged
BWC test workflow #1603
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Example test results: |
|
Related OpenSearch Dashboards PR: opensearch-project/OpenSearch-Dashboards#1190 |
kavilla
commented
Feb 8, 2022
| ) | ||
|
|
||
| def get_cmd(self, script, security, manifest_build_location): | ||
| return f"{script}" |
There was a problem hiding this comment.
This should be updated but current bwctest.sh doesn't accept other arguments: https://github.com/opensearch-project/anomaly-detection/blob/main/bwctest.sh
Adding backwards compatability workflow for OpenSearch and OpenSearch Dashboards. Issue: opensearch-project#705 Signed-off-by: Kawika Avilla <[email protected]>
kavilla
force-pushed
the
avillk/bwc_test
branch
from
ea3a423 to
f3b674c
Compare
kavilla
requested review from
saratvemulapalli,
tianleh,
dblock and
peternied
dblock
approved these changes
Feb 8, 2022
| with TemporaryDirectory(keep=self.args.keep, chdir=True) as work_dir: | ||
| all_results = TestSuiteResults() | ||
| for component in self.components.select(focus=self.args.component): | ||
| if component.name in self.test_manifest.components: |
There was a problem hiding this comment.
Not for this PR, but we do a similar workflow in many places and I don't love the double-select here, first pass --component name to pick a component, but then we also only want components that have a test configuration.
One option would be to fail if self.args.component not in self.test_manifest.components with an explicit error, and refactor components.select to take an array of components, i.e. self.components.select(focus=self.test_manifest.components).
Another, possibly better, could be to do self.components.select(focus=self.args.component, configs=self.test_manifest.components, key: "bwc_test") that would return a set of tuples of component + configuration. The code then becomes quite clean:
for component, config in self.components.select(focus=self.args.component, configs: self.test_manifest.components, key: "bwc_test"):
test_suite = ...
...There was a problem hiding this comment.
Created a cleanup issue here: #1604 and assigned myself.
Signed-off-by: Kawika Avilla <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #1603 +/- ##
============================================
+ Coverage 94.50% 94.52% +0.01%
Complexity 13 13
============================================
Files 153 162 +9
Lines 3296 3433 +137
Branches 13 21 +8
============================================
+ Hits 3115 3245 +130
- Misses 172 185 +13
+ Partials 9 3 -6
Continue to review full report at Codecov.
|
dblock
approved these changes
Feb 8, 2022
peternied
reviewed
Feb 8, 2022
| test-configs: | ||
| - with-security | ||
| - without-security | ||
| schema-version: '1.0' |
There was a problem hiding this comment.
Nit; Can we move this to the top?
peternied
reviewed
Feb 8, 2022
| Or run BWC tests against an existing build. | ||
|
|
||
| ```bash | ||
| ./test.sh bwc-test manifests/1.3.0/opensearch-1.3.0-test.yml --paths opensearch=https://ci.opensearch.org/ci/dbc/bundle-build/1.2.0/869/linux/x64 # looks for https://.../builds/opensearch/manifest.yml and https://.../dist/opensearch/manifest.yml |
There was a problem hiding this comment.
Thanks for the great documentation
peternied
reviewed
Feb 8, 2022
| @@ -0,0 +1,39 @@ | |||
| # SPDX-License-Identifier: Apache-2.0 | |||
There was a problem hiding this comment.
Nit: this class is a whole lot of stuff for effectively two lines of differentiated behavior. I don't think there is an easier way but food for thought
peternied
reviewed
Feb 8, 2022
| mock_bundle_manifest = MagicMock() | ||
| mock_bundle_manifest.components = mock_components | ||
|
|
||
| mock_build_manifest = MagicMock() |
There was a problem hiding this comment.
Nit; Looks like you setup mock manifests a couple of times over, might be worthwhile to add MockBuildManifest/MockTestManifest that could condense these unit tests, or at least let them reuse the same setup logic
There was a problem hiding this comment.
Note; even if they used a 'canned' construction pattern and then you modified them afterward to fit one test or another that could be useful
peternied
approved these changes
Feb 8, 2022
There was a problem hiding this comment.
Great contribution!
We've got an outstanding item #1512 to make sure the test workflows have typing information - do you think you could see about adding in typing info in your new classes? If you'd like to pickup that issue separately that works for me too.
Not sure what you quiet mean here. Do you have an example? If it requires updates, then probably will need to do this in the integ tests as well. Which might be beneficial to create another clean up task that I can do to get the bwc tests and integ tests aligned. |
|
I've merged your change, checkout issue #1237 for a detailed explanation with some example PRs |
Will do and will handle some of the clean ups you suggested |
peterzhuamazon
pushed a commit
to peterzhuamazon/opensearch-build
that referenced
this pull request
Feb 16, 2022
* BWC test workflow Adding backwards compatability workflow for OpenSearch and OpenSearch Dashboards. Issue: opensearch-project#705 Signed-off-by: Kawika Avilla <[email protected]> * [BWC] Update docs and return cmd Signed-off-by: Kawika Avilla <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adding backwards compatability workflow for OpenSearch and OpenSearch
Dashboards.
Signed-off-by: Kawika Avilla [email protected]
Issues Resolved
#705
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.