Skip to content
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

Redesign the migration regression cases #15154

Closed
wants to merge 1 commit into from

Conversation

chcao
Copy link
Contributor

@chcao chcao commented Jun 24, 2022

We redesign the migration regression cases that spliting a long case into
several cases: after migration, publsh a qcow, and then do regression
tests with the qcow; for the regression tests, we use INCLUDE_MODULES
setting to load the regression test modules. The reason that fix the
service check is we check the upgraded services on the second case, it can
not check the result in the first job.

@github-actions
Copy link

Great PR! Please pay attention to the following items before merging:

Files matching lib/**.pm:

  • Consider adding or extending unit tests in t/

This is an automatically generated QA checklist based on modified files

Copy link
Contributor

@lemon-suse lemon-suse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this need more investigation.

  1. Regression module have module dependency, how to solve it? If need more module combination for the migration qcow, need investigate whether the redesign is valuable.
  2. Need investigate whether to split to two jobs is valuable to do it.
  3. I don't think change the logic of service check is needed. If you want to change it for a specific change please add setting to control.
    From my opinion, we just need split the regression test into more small group is enough, do not need so big change

@chcao chcao force-pushed the migration_regression_yaml branch from 9ced6c5 to 355227e Compare June 24, 2022 07:48
@chcao
Copy link
Contributor Author

chcao commented Jun 24, 2022

I think this need more investigation.

1. Regression module have module dependency, how to solve it? If need more module combination for the migration qcow, need investigate whether the redesign is valuable.

If any modules are needed by regression tests, add the module in the migration case would solve it. BTW, the uploaded qcow is registered, so the environment is the same as before.

2. Need investigate whether to split to two jobs is valuable to do it.

This is what I did recently, splitting to several jobs would save the time that running the regression tests one by one.

3. I don't think change the logic of service check is needed. If you want to change it for a specific change please add setting to control.

I didn't change the logic of service check. I just put the checking of the upgraded services into the second case.

   From my opinion, we just need split the regression test into more small group is enough, do not need so big change

@chcao chcao force-pushed the migration_regression_yaml branch 5 times, most recently from ed33061 to cf811ab Compare July 6, 2022 03:29
We redesign the migration regression cases that spliting a long case into
several cases: after migration, publsh a qcow, and then do regression
tests with the qcow; for the regression tests, we use INCLUDE_MODULES
setting to load the regression test modules. The reason that fix the
service check is we check the upgraded services on the second case, it can
not check the result in the first job.
@chcao chcao force-pushed the migration_regression_yaml branch from cf811ab to 9228e52 Compare July 8, 2022 02:17
@lemon-suse
Copy link
Contributor

lemon-suse commented Jul 8, 2022

The PR seems works while base on the solution will cause lost test coverage, and it's benefit can be realize with debug method. I will reserve my opinion, PO & SM can decide whether to use this solution.

@chcao chcao closed this Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants