Merge lp://staging/~dpb/latch-test/dont-test-so-much into lp://staging/latch-test

Proposed by David Britton
Status: Merged
Approved by: David Britton
Approved revision: 21
Merged at revision: 14
Proposed branch: lp://staging/~dpb/latch-test/dont-test-so-much
Merge into: lp://staging/latch-test
Diff against target: 237 lines (+98/-24)
2 files modified
latch.py (+43/-7)
test_latch.py (+55/-17)
To merge this branch: bzr merge lp://staging/~dpb/latch-test/dont-test-so-much
Reviewer Review Type Date Requested Status
Данило Шеган (community) Approve
🤖 Landscape Builder test results Approve
Benji York (community) Approve
Review via email: mp+290102@code.staging.launchpad.net

Commit message

When the testing starts, set review to Abstain, when scanner runs, don't launch more tests if it notices 'Abstain' on the bot's slot.

Description of the change

When the testing starts, set review to Abstain, when scanner runs, don't launch
more tests if it notices 'Abstain' on the bot's slot.

Testing instructions:

it's not super easy, since you really need an LP project and an MP, but please
feel free to check out this MP, which I used for testing:

https://code.launchpad.net/~davidpbritton/dpb-code/test-8/+merge/214602

Notice the abstain was put there by 'test', and the 'scan' job ignored this MP
because of it. Here is the config I use for testing on the 'dpb-code' project,
and you could set up anything similar on your project as well.

[lp:dpb-code]
verify_command = sleep 60; /bin/false
voting_criteria = Approve >= 1, Disapprove == 0
commit_message_template = Merged <branch_name>. [f=<bugs_fixed>] [r=<approved_by_nicks>]\n<commit_message>
artifacts = tarmac.conf

To post a comment you must log in.
Revision history for this message
🤖 Landscape Builder (landscape-builder) wrote :

Command: make test
Result: Success
Revno: 15
Branch: lp:~davidpbritton/latch-test/dont-test-so-much
Jenkins: https://ci.lscape.net/job/latch-test/4066/

review: Approve (test results)
Revision history for this message
Benji York (benji) wrote :

Given the complexity of setting up a test environment, the low probability of damage due to a bug in this branch, and the fact that misbehavior will be obvious once landed, I did not run the code in this branch.

The code looks good. I had a couple questions/small suggestions that should be addressed before landing, but I'm sure the responses will be land-worthy, so I'm approving presuming the tweaks.

review: Approve
16. By David Britton

revert change of 'config' file, accident.

17. By David Britton

turn add_votes into add_vote and call multiple times to clean up usage.

Revision history for this message
David Britton (dpb) :
Revision history for this message
🤖 Landscape Builder (landscape-builder) wrote :

Command: make test
Result: Success
Revno: 17
Branch: lp:~davidpbritton/latch-test/dont-test-so-much
Jenkins: https://ci.lscape.net/job/latch-test/4079/

review: Approve (test results)
18. By David Britton

s/with multiple//

Revision history for this message
🤖 Landscape Builder (landscape-builder) wrote :

Command: make test
Result: Success
Revno: 18
Branch: lp:~davidpbritton/latch-test/dont-test-so-much
Jenkins: https://ci.lscape.net/job/latch-test/4080/

review: Approve (test results)
Revision history for this message
Данило Шеган (danilo) wrote :

I am getting a lint error on "make lint":

./latch.py:197:33: E901 SyntaxError: invalid syntax
./test_latch.py:73:1: E302 expected 2 blank lines, found 1
Makefile:8: recipe for target 'lint' failed
make: *** [lint] Error 1

This might be due to python3 being default on xenial, but it's easy enough to fix, so please do that (wrap print parameters on line 197 inside parentheses).

A few other nits inline, but overall a nice change: thanks for doing this work!

review: Approve
19. By David Britton

s/thisvote/this_vote/

20. By David Britton

danilo[diff]: use vote_lookup, not 'Abstain' directly.

21. By David Britton

danilo[diff]: restructure conditional

Revision history for this message
David Britton (dpb) :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: