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

Add a new parameter to support not schedule parent test suite #2224

Merged
merged 1 commit into from
Aug 1, 2019

Conversation

Amrysliu
Copy link
Contributor

@Amrysliu Amrysliu commented Jul 29, 2019

When users use 'client isos post' to schedule test suite, if they specify
'_NO_TRIGGER_DEPENDENCY=1', do not schedule parent test suites that defined by
'START_AFTER_TEST'.

See: https://progress.opensuse.org/issues/30730

@Amrysliu
Copy link
Contributor Author

I am not sure if this solution is correct, so I defined it as 'WIP'. There is a problem in it, after creating jobs, the Dependencies (In web UI, the label of jobs detail) does not show the relationship of the parent test suite. I have no idea how to fix it. Should I query the jobs database and set up the last successful job (that is depend on) with the child test suite?

@codecov
Copy link

codecov bot commented Jul 29, 2019

Codecov Report

Merging #2224 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2224      +/-   ##
==========================================
+ Coverage   85.81%   85.85%   +0.04%     
==========================================
  Files         162      162              
  Lines       10738    10742       +4     
==========================================
+ Hits         9215     9223       +8     
+ Misses       1523     1519       -4
Impacted Files Coverage Δ
lib/OpenQA/Schema/Result/ScheduledProducts.pm 97.2% <100%> (+0.04%) ⬆️
lib/OpenQA/WebSockets.pm 92.85% <0%> (-1.43%) ⬇️
lib/OpenQA/Worker/Engines/isotovideo.pm 93.68% <0%> (-0.98%) ⬇️
lib/OpenQA/Worker/Settings.pm 98.03% <0%> (+13.72%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ce7ed5...8c5ed75. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 29, 2019

Codecov Report

Merging #2224 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2224      +/-   ##
==========================================
+ Coverage   85.88%   85.95%   +0.07%     
==========================================
  Files         164      164              
  Lines       10803    10807       +4     
==========================================
+ Hits         9278     9289      +11     
+ Misses       1525     1518       -7
Impacted Files Coverage Δ
lib/OpenQA/Schema/Result/ScheduledProducts.pm 97.2% <100%> (+0.04%) ⬆️
lib/OpenQA/Worker/Settings.pm 98.03% <0%> (+13.72%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59d9cc5...3e1ac84. Read the comment docs.

@okurz
Copy link
Member

okurz commented Jul 29, 2019

As e.g. openqa-clone-job already supports parameters like --skip-chained-deps I suggest to use at least the same words unless this new feature is actually doing something completely different.

There is a problem in it, after creating jobs, the Dependencies (In web UI, the label of jobs detail) does not show the relationship of the parent test suite. I have no idea how to fix it. Should I query the jobs database and set up the last successful job (that is depend on) with the child test suite?

IMHO it is ok if there is no dependency visualized as there actually are no dependencies between the jobs, only a dependency to the used asset and I am sure that one is still shown in the "Download" tab.

@Amrysliu
Copy link
Contributor Author

Amrysliu commented Jul 30, 2019

As e.g. openqa-clone-job already supports parameters like --skip-chained-deps I suggest to use at least the same words unless this new feature is actually doing something completely different.

To be consistent with existed parameters (such as _ONLY_OBSOLETE_SAME_BUILD), I used _SKIP_CHAINED_DEPS as the new parameter. Do you think it is ok?

@okurz
Copy link
Member

okurz commented Jul 30, 2019

Yes, I think this is a good choice, only that your github quoting style is a bit off ;)

@Amrysliu Amrysliu changed the title WIP: Add a new parameter to support not trigger dependency test suite Add a new parameter to support not trigger dependency test suite Jul 30, 2019
t/api/02-iso.t Outdated Show resolved Hide resolved
t/api/02-iso.t Outdated Show resolved Hide resolved
lib/OpenQA/Schema/Result/ScheduledProducts.pm Outdated Show resolved Hide resolved
@Amrysliu Amrysliu force-pushed the fix_30730 branch 2 times, most recently from fcf9f3e to f8e2c49 Compare July 31, 2019 06:58
@Amrysliu Amrysliu requested a review from Martchus July 31, 2019 07:57
@@ -644,6 +644,8 @@ mean existing jobs for earlier builds with the same DISTRI and VERSION are
no longer interesting, but you still want to be able to re-submit jobs for a
build and have existing jobs for the exact same build obsoleted.

_SKIP_CHAINED_DEPS:: Do not trigger parent test suites that specified in START_AFTER_TEST.
Copy link
Member

Choose a reason for hiding this comment

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

s/that specified/which are specified/

Copy link
Contributor

Choose a reason for hiding this comment

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

I also don't like the word trigger. It is very unspecific (it has too many different meanings to be meaningful).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changing it to "Do not schedule parent test suites which are specified in START_AFTER_TEST.", is it okay?

lib/OpenQA/Schema/Result/ScheduledProducts.pm Outdated Show resolved Hide resolved
lib/OpenQA/Schema/Result/ScheduledProducts.pm Outdated Show resolved Hide resolved
t/api/02-iso.t Outdated Show resolved Hide resolved
t/api/02-iso.t Outdated Show resolved Hide resolved
@Amrysliu Amrysliu force-pushed the fix_30730 branch 2 times, most recently from c96f0b9 to eb20521 Compare August 1, 2019 02:40
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

Looks OK change-wise but your commit message still includes WIP. Want to add more or do you consider your changes ready?

lib/OpenQA/Schema/Result/ScheduledProducts.pm Outdated Show resolved Hide resolved
When users use 'client isos post' to schedule test suite, if they specify
'_SKIP_CHAINED_DEPS=1', do not schedule parent test suites that defined by
'START_AFTER_TEST'.

See: https://progress.opensuse.org/issues/30730
@Amrysliu Amrysliu changed the title Add a new parameter to support not trigger dependency test suite Adding a new parameter to support not schedule parent test suite Aug 1, 2019
@Amrysliu
Copy link
Contributor Author

Amrysliu commented Aug 1, 2019

Looks OK change-wise but your commit message still includes WIP. Want to add more or do you consider your changes ready?

Thanks for the mention. I modified the commit message.

@Amrysliu Amrysliu changed the title Adding a new parameter to support not schedule parent test suite Add a new parameter to support not schedule parent test suite Aug 1, 2019
@okurz okurz merged commit ac734d5 into os-autoinst:master Aug 1, 2019
coolo pushed a commit that referenced this pull request Aug 1, 2019
commit ac734d5
Merge: 59d9cc5 3e1ac84
Author:     Oliver Kurz <okurz@suse.de>
AuthorDate: Thu Aug 1 11:20:52 2019 +0200
Commit:     GitHub <noreply@github.com>
CommitDate: Thu Aug 1 11:20:52 2019 +0200

    Merge pull request #2224 from Amrysliu/fix_30730

    Add a new parameter to support not schedule parent test suite
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants