Merge lp://qastaging/~kissiel/checkbox/fix-1391774-required-field-in-tp into lp://qastaging/checkbox
Status: | Work in progress |
---|---|
Proposed branch: | lp://qastaging/~kissiel/checkbox/fix-1391774-required-field-in-tp |
Merge into: | lp://qastaging/checkbox |
Diff against target: |
206 lines (+82/-7) 4 files modified
checkbox-ng/checkbox_ng/commands/newcli.py (+5/-1) plainbox/docs/manpages/plainbox-test-plan-units.rst (+9/-0) plainbox/plainbox/impl/unit/test_testplan.py (+33/-2) plainbox/plainbox/impl/unit/testplan.py (+35/-4) |
To merge this branch: | bzr merge lp://qastaging/~kissiel/checkbox/fix-1391774-required-field-in-tp |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Checkbox Developers | Pending | ||
Review via email: mp+261860@code.qastaging.launchpad.net |
Description of the change
This MR brings 'required' field to test plan unit.
Jobs matching any pattern from the 'required' field will get always get executed REGARDLESS of the selection made. This change set doesn't make any changes in existing providers, but it paves the way for whitelist -> test plan conversion.
b2b06a3 plainbox:
0b621a3 plainbox:
bd0325e checkbox-
1c555e5 plainbxo:
Unmerged revisions
- 3831. By Maciej Kisielewski
-
plainbxo:
docs:manpages: update testplan - add 'required' field info Signed-off-by: Maciej Kisielewski <email address hidden>
- 3830. By Maciej Kisielewski
-
checkbox-
ng:commands: make 'normal run' run required jobs This patch makes checkbox-ng run required jobs when doing normal sequence.
Fixes: https:/
/bugs.launchpad .net/plainbox- provider- checkbox/ +bug/1463921 Signed-off-by: Maciej Kisielewski <email address hidden>
- 3829. By Maciej Kisielewski
-
plainbox:
impl:unit: testplan: add get_required_ jobs_qualifier to testplan This patch adds testplan method to get qualifier that corresponds to jobs
pointed by the 'required' field.Signed-off-by: Maciej Kisielewski <email address hidden>
- 3828. By Maciej Kisielewski
-
plainbox:
impl:unit: testplan: add 'required' field This patch adds 'required' field to the testplan that specifies jobs that
should ALWAYS be run, regardless of what user selects.Signed-off-by: Maciej Kisielewski <email address hidden>
General comment, don't include the "impl" part in plainbox commit messages.
Push stuff _earlier_ before stuff is done as any re-design changes are cheaper.
Please fix typos in commit messages below.
Please patch plainbox to run the mandatory part if plainbox run -T is used, then you probably won't have to patch checkbox-ng at all.
I want to see a design decision on what happens if a job is selected by both normal and mandatory parts. I think it should trigger a warning from the test plan validator and that this job should be impossible to de-select in checkbox- ng/gui/ touch later.
Looking at the get_$newthing_ qualifier( ) it seems like a copy-paste of the other one, perhaps we want a new private method and have all the old methods and the new method call that instead.
Please expand the manual page to include some examples. I think we should include a very useful certification example that says "if you want a test plan that works for certification purpose, please add this snippet "required: ..."
Oh, and the word required is better but I think I still prefer mandatory as it's used similarly in other products. I'd put my $0.90 on mandatory-include instead.
More reviews next week. Thanks.