Run OSD integ test ci-groups in parallel #5179
Conversation
| if self.args.ci_group: | ||
| orig_component_name = component.name | ||
| for i in range(1, test_config.integ_test['ci-groups'] + 1): | ||
| component.name = f"{orig_component_name}-ci-group-{i}" | ||
| test_suite = self.__create_test_suite__(component, test_config, work_dir.path) | ||
| test_results = test_suite.execute_tests() | ||
| [self.test_recorder.test_results_logs.generate_component_yml(result_data) for result_data in test_suite.result_data] | ||
| all_results.append(component.name, test_results) | ||
| component.name = f"{orig_component_name}-ci-group-{self.args.ci_group}" | ||
| test_suite = self.__create_test_suite__(component, test_config, work_dir.path) | ||
| test_results = test_suite.execute_tests() | ||
| [self.test_recorder.test_results_logs.generate_component_yml(result_data) for result_data in test_suite.result_data] | ||
| all_results.append(component.name, test_results) |
There was a problem hiding this comment.
Looks like if and else block has a duplicate code. Does the below recommendation makes sense? Only change component-name if ci-group is present. Everything goes on as before.
if self.args.ci_group:
orig_component_name = component.name
component.name = f"{orig_component_name}-ci-group-{self.args.ci_group}"
test_suite = self.__create_test_suite__(component, test_config, work_dir.path)
test_results = test_suite.execute_tests()
[self.test_recorder.test_results_logs.generate_component_yml(result_data) for result_data in test_suite.result_data]
all_results.append(component.name, test_results)
| @@ -149,8 +151,6 @@ def test_with_integ_test_ci_groups(self, mock_temp: Mock, mock_test_recorder: Mo | |||
| results = runner.run() | |||
|
|
|||
| self.assertEqual(results["sql-ci-group-1"], mock_test_results) | |||
| self.assertEqual(results["sql-ci-group-2"], mock_test_results) | |||
There was a problem hiding this comment.
Can we check his assertions for given ci-group number?
There was a problem hiding this comment.
This test is checking the assertion when the ci-group parameter is passed with value 1.
I just deleted the previous implementation of running all ci-groups in the manifest file.
| @@ -0,0 +1,376 @@ | |||
| /* | |||
There was a problem hiding this comment.
Can you please make changes in current integ-test workflow so that diff is easily visible?
There was a problem hiding this comment.
I am proposing a dedicated job for OSD integ test because adding this ci-group changes in the existing job will be hacky and not clean. Running ci-groups and different components are technically two different use-cases.
This can be done in existing file but will make it clunky and hacky. For e.g, we don't need build manifest related stuff for ci-group processing and will result in a lot of if-else.
There was a problem hiding this comment.
How about putting it in a library and calling the library instead. Seeing a lot of duplicate code and also worried about the maintenance like changing docker image names in both files, agent names, etc.
There was a problem hiding this comment.
Refactored to use the same job file.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5179 +/- ##
==========================================
- Coverage 92.12% 92.11% -0.01%
==========================================
Files 197 197
Lines 6817 6814 -3
==========================================
- Hits 6280 6277 -3
Misses 537 537 ☔ View full report in Codecov by Sentry. |
rishabh6788
force-pushed
the
main
branch
5 times, most recently
from
9ecbd6d
to
5cfaf48
Compare
Signed-off-by: Rishabh Singh <[email protected]>
rishabh6788
requested review from
dblock,
peterzhuamazon,
zelinh,
prudhvigodithi,
Divyaasm and
tianleh
as code owners
Description
Run OSD integ test ci-groups in parallel.
This change proposes a dedicated job to run OSD integration test ci-groups in parallel.
The existing job can add a check to remove OSD component from list and run as usual.
Issues Resolved
#4944
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.