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

Fix rounding error when spawning users from multiple locust classes #1113

Merged
merged 2 commits into from
Oct 21, 2019

Conversation

heyman
Copy link
Member

@heyman heyman commented Oct 20, 2019

This should fix #848 and is an alternative implementation to #860. It's simpler but less sophisticated.

With this PR one could still get an uneven distribution of Locust classes when killing locusts. For example if one has multiple Locust classes and then kills a single user one by one by changing the number of locusts multiple times and decreasing the number by one each time. I don't think this is a big issue. It could be fixed later by rewriting the logic of the LocustRunner.kill_locusts() method to - instead of using weight_locusts() to get a list of classes to remove - use weight_locusts() to get a list of the new distribution and then diffing the current distribution to the new one and killing the locusts needed to achieve the new distribution.

@codecov
Copy link

codecov bot commented Oct 20, 2019

Codecov Report

Merging #1113 into master will decrease coverage by 0.21%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1113      +/-   ##
==========================================
- Coverage   73.16%   72.94%   -0.22%     
==========================================
  Files          18       18              
  Lines        1755     1763       +8     
  Branches      257      261       +4     
==========================================
+ Hits         1284     1286       +2     
- Misses        408      418      +10     
+ Partials       63       59       -4
Impacted Files Coverage Δ
locust/runners.py 54.18% <100%> (-1.24%) ⬇️
locust/core.py 85.26% <0%> (ø) ⬆️
locust/contrib/fasthttp.py 94.11% <0%> (+1.17%) ⬆️

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 40f64e0...aa77054. Read the comment docs.

@cyberw
Copy link
Collaborator

cyberw commented Oct 20, 2019

LGTM. Not loving the variable name "amount", but that was there before the PR so whatever :)

@heyman heyman merged commit a7d6485 into master Oct 21, 2019
@heyman
Copy link
Member Author

heyman commented Oct 21, 2019

Not loving the variable name "amount", but that was there before the PR so whatever :)

I agree.

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.

Tests fail to run when providing a small number of clients
2 participants