Worker quitting then stopping via web UI bug fix

[Trouv]

Fixes #1279

The issue is that when all workers quit, the test is stopped AND the client_listener greenlet stops. So, when the user presses Stop after the worker quits, the state is set to STATE_STOPPING, but the client_listener will never set it from STATE_STOPPING to STATE_STOPPED again. This seems to be why the UI gets confused on refresh.

Ideally, I think all workers quitting would update the UI to its stopped form and the stop button would disappear. Even then, this if statement (or something similar) is probably still good to have for idempotency's sake.

[codecov]

Codecov Report

Merging #1324 into master will increase coverage by 0.20%.
The diff coverage is 63.15%.

@@            Coverage Diff             @@
##           master    #1324      +/-   ##
==========================================
+ Coverage   80.82%   81.02%   +0.20%     
==========================================
  Files          24       24              
  Lines        2169     2182      +13     
  Branches      329      332       +3     
==========================================
+ Hits         1753     1768      +15     
- Misses        329      331       +2     
+ Partials       87       83       -4     

Impacted Files	Coverage Δ
locust/runners.py	80.42% <63.15%> (+0.98%)	⬆️

---

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 54b7977...59f117f. Read the comment docs.

[Trouv]

Or would something like this be better?

[cyberw]

I think we should do both fixes! The ui part for better clarity for the user, and the other part for more robustness.

[Trouv]

Sounds good! I added the second fix I linked in. I also noticed that test_stop wasn't being fired when the workers quit, so now stop() is called when the last worker quits.

[heyman]

Nice!

Could you write tests for this as well?

Or would something like this be better?

I think it's fine if clicking the stop button when in a stopped/stopping state results in a no-op. Also, message is actually never displayed to the user AFAIK, so currently those extra lines doesn't really do anything. I believe message was added some time in Locust's infancy and was never actually used (so we should probably remove it in another PR).

[Trouv]

I believe I've addressed the issue in a more complete way now. I noticed that making the worker go missing instead of quitting would still cause the bug. So, I moved things around a little bit.

I think the real core of the issue was that setting it to STATE_STOPPED was handled in client_listener. When client_listener has nothing to listen to, the state will never be updated to STOPPED. Also, setting it to STATE_STOPPED isn't directly related to dealing with the messages from the workers, it has more to do with the state of the self.clients.

I moved setting the STATE_STOPPED to the heartbeat_worker function instead, since it doesn't hang when there are no messages coming in. I think it fits here since this function is used to check on the state of self.clients already. I also moved checking the # of workers remaining to here, and made this compatible with missing workers.

This broke one test case that was using sleep() without sending a mock heartbeat, so I've updated it to do so.

I still plan to write tests for this soon, and maybe look into why refreshing when the state is TEST_STOPPING makes it look so weird in the first place.

[heyman]

Looks good! 👍

One last thing would be to speed up the new tests (see my inline comments).

[Trouv]

Cool, thanks for your help!

[heyman]

Thanks for the contribution!