r-'ing a testing PR will not cancel it

[kennytm]

Repro steps:

1. r+ a PR
2. Wait for it to enter the "⏳ Testing commit" (pending) phase
3. r- it before the test completes

Expected behavior:

• The test is canceled and we move to the next PR in the queue immediately. (Note: currently it will result in "💥 Test timed out" but that's a separate bug and harmless.)

Actual:

• The test continues, and when the test completed, homu will comment (without merging):

  ☀️ Test successful - status-travis
  State: approved= try=False

  and then move on to the next PR.

This new behavior is introduced after (excluding) 0979fe7, so still cc @alexrs 😄

[alexrs]

Thanks for reporting it! I'll look into it.

[alexrs]

@kennytm Can you show me a PR where this happens?

[kennytm]

@alexrs It is a bit messy but you could check https://github.com/kennytm/test-term-coloring/pull/7#issuecomment-376501042 and https://github.com/kennytm/test-term-coloring/pull/8#issuecomment-376501042.

• 2018-03-27T12:08:03Z: r+ on PR 7 issued
• 2018-03-27T12:08:05Z: r+ on PR 7 acknowledged
• 2018-03-27T12:08:09Z: r+ on PR 8 issued
• 2018-03-27T12:08:10Z: r+ on PR 8 acknowledged
• 2018-03-27T12:08:10Z: Testing on PR 7 began
• 2018-03-27T12:08:22Z: r- on PR 7 issued
• 2018-03-27T12:09:40Z: Testing on PR 7 completed
• 2018-03-27T12:09:44Z: Testing on PR 8 began
• 2018-03-27T12:12:33Z: Testing on PR 8 completed

The testing on PR 8 should begin right after 08:22Z.

[alexrs]

I've been busy lately, but I'll look into this ASAP.

[KiChjang]

I believe that if you add a force along with r-, homu would have the correct behavior, so we can instead make r- imply force, but that doesn't sound very safe to me.

[alexrs]

I've been comparing 0979fe7with the latest master (https://github.com/servo/homu/compare/0979fe7151a43fabd4925187ab1f97100bf4d400...master) and I don't see where this problem could have been introduced...