new parameter for user to specify a ref for opensearch-dashboards-functional test #4260
Conversation
Signed-off-by: chawinphat <[email protected]>
Signed-off-by: chawinphat <[email protected]>
Signed-off-by: chawinphat <[email protected]>
Signed-off-by: chawinphat <[email protected]>
chawinphat
requested review from
dblock,
peterzhuamazon,
bbarani,
gaiksaya,
rishabh6788,
zelinh,
jordarlu,
prudhvigodithi,
Divyaasm and
tianleh
as code owners
Signed-off-by: chawinphat <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4260 +/- ##
==========================================
+ Coverage 91.18% 91.19% +0.01%
==========================================
Files 189 189
Lines 6146 6153 +7
==========================================
+ Hits 5604 5611 +7
Misses 542 542 ☔ View full report in Codecov by Sentry. |
Signed-off-by: chawinphat <[email protected]>
chawinphat
force-pushed
the
dash-func-test
branch
from
83c73a4 to
034f3aa
Compare
There was a problem hiding this comment.
Please add test cases for this. See https://github.com/opensearch-project/opensearch-build/blob/main/tests/tests_test_workflow/test_test_args.py
src/test_workflow/integ_test/integ_test_suite_opensearch_dashboards.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Sayali Gaikawad <[email protected]> Signed-off-by: Teddy Tankuranand <[email protected]>
Signed-off-by: chawinphat <[email protected]>
Signed-off-by: chawinphat <[email protected]>
Signed-off-by: chawinphat <[email protected]>
Signed-off-by: chawinphat <[email protected]>
There was a problem hiding this comment.
LGTM! Adding @peterzhuamazon for another review.
We might also need to update the test_workflow documentation and jenkins workflows to accommodate this change.
| @@ -57,10 +58,13 @@ def __init__( | |||
| ) | |||
|
|
|||
| # Integ-tests for OSD now clones FunctionalTestDashboards Repository by default and points to integtest.sh from FunctionalTestDashboards for all OSD plugins | |||
| functional_test_dashboards_ref = ft_repo_ref | |||
There was a problem hiding this comment.
Suggest getting consistent on the naming:
- If it is
functional_test_repo, then use it consistently and avoidft_repo. - Keep it consistent with
opensearch_dashboardsinstead ofdashboards. (ex: build_manifest_opensearch_dashboards)
There was a problem hiding this comment.
@peterzhuamazon just saw these comments. Will work on it soon.
| @@ -33,6 +34,7 @@ def __init__(self) -> None: | |||
| parser.add_argument("--test-run-id", type=int, help="The unique execution id for the test") | |||
| parser.add_argument("--component", type=str, dest="components", nargs='*', help="Test a specific component or components instead of the entire distribution.") | |||
| parser.add_argument("--keep", dest="keep", action="store_true", help="Do not delete the working temporary directory.") | |||
| parser.add_argument("--ft-repo-ref", type=str, default=None, dest="ft_repo_ref", help="Specify GitHub ref for functionalTestDashboards repo to override default one from build manifest.") | |||
There was a problem hiding this comment.
Suggest --functional-test-repo-ref, or anything that is consistent across the board just like mentioned above.
| def test_ft_repo_ref(self) -> None: | ||
| test_args = TestArgs() | ||
| self.assertEqual(test_args.ft_repo_ref, "main") |
There was a problem hiding this comment.
Is there any other test with user provided ref instead of default to main?
|
Thanks for contribution @chawinphat. To my understanding, this PR added a new parameter for user to specify a ref. If so, then the title should be corrected as it is not Thanks. |
chawinphat
changed the title
chawinphat
changed the title
|
Close due to no activities and replace by: |
Description
Added a new parameter to test_args "--ft-repo-ref" which allows users to specify which branch they want to checkout when testing opensearch-dashboards-functional test repo.
Issues Resolved
Closes #4057
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.