-
Notifications
You must be signed in to change notification settings - Fork 3k
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
ft: LoadTestShapes with custom user classes #2181
ft: LoadTestShapes with custom user classes #2181
Conversation
Sounds like a good idea! I'll give some comments, but then we also need:
I'd be really interested to hear @mboutet and @max-rocket-internet 's opinions on this too. |
I think this is also what was mentioned in #2151 |
If it doesn't affect existing uses of This PR looks quite simple but I don't have much understanding of how a load test works with multiple |
Yeah it's quite similar but an approach from a different direction. So using multiple users is working quite well, even without this PR. Our setup is just quite complex, also ensuring that some users have been spawned before others. |
It's a great feature indeed! Make sure to add tests. In particular, add tests in |
I added some tests. I will additionally add those for addition and removal of workers. The ramp-down, behaviour should be unchanged, as the FYI @cyberw , @max-rocket-internet , @mboutet I had some issues getting an environment to work as expected. In the end I needed to copy all dependencies from the |
What was your error message? Unfortunately I only have an intel mac so I cant test it out for myself... |
@cyberw Those are only the last log messages, because the whole would be quite long. But if wanted I can also post the whole thing.
|
And one question, for running the |
|
Yes. You may also need to add the things listed in tox.ini under testenv:
|
@cyberw Thanks that helped a lot. I had some issues with the relative imports done in some of the tests, but after resolving them locally nearly everything seems to work. I have one failing test and am pretty clueless about it. I would be happy to get some guidance from you guys :)
This is the only one who's failing. So functioning wise everything is working as expected (Tested it with a custom_shape locust file from the examples in standalone and distributed mode). |
Try reordering the assertions to check stderr or exit code first, that may give more info... |
This is the error:
|
And this is the locust file from the test.
|
@cyberw Okay so the test Is this a bug in the test? The working test looks like this:
|
Hmm... sorry, I cant really think of why it should fail like that, and I dont see any error in the test. Is it possible that your changes actually broke something? |
@cyberw Okay so with some thought I found out why. Yeah there were also some changes that broke it. But this issue has two sides:
This is the code how it was. The thing is that content in this case is including 2 structures of a locust file. The one included in As I implemented the feature I forgot that each time I update the user_classes I also need to sort them. I changed that, and locally it works as expected. But still if the user would not be called |
Ah, I see now. The problem is that UserSubclass has no tasks tagged with It is starting to look pretty good now. Do we need a distributed test? Probably not, as workers wont know any difference.. |
locust/runners.py
Outdated
@@ -486,6 +494,9 @@ def _start(self, user_count: int, spawn_rate: float, wait: bool = False) -> None | |||
if wait and user_count - self.user_count > spawn_rate: | |||
raise ValueError("wait is True but the amount of users to add is greater than the spawn rate") | |||
|
|||
# if user_classes is None: | |||
# user_classes = self.user_classes | |||
|
|||
for user_class in self.user_classes: | |||
if self.environment.host: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if we combine a -H/--host parameter with shape-configured Users? I guess that will still work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So normally it should. I'll try it out tomorrow :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cyberw So I tried it with a few example locust files, and it seemed to work.
If you want to be sure, you can also try it out. :)
@cyberw Do you want to resolve the threads yourself, or should I do it? :) |
Go ahead and resolve as you fix stuff, I'm too lazy :) |
I made a PR (#2186) to log a warning when tag filtering made it so that there were no tasks left. Probably not the most beautiful solution, but should be better than before. |
looking really good now. just need to rename Runner.shape_last_state to shape_last_tick as well, to be consistent (sorry for derailing your PR a little bit :) |
Don't worry, code style and consistency is important. @cyberw An off-topic question for the tests. Is there a reason that in some of the tests the imports are relative and not absolute? Just out of curiosity :) |
Tbh, I dont know why, could be just other people writing them :) Thanks for this PR, merging now! |
Thanks for all the comments @cyberw :) Do you already know when a new release is going to be created? |
Done. 2.12.0 should be available in less than 10 minutes. |
Hey Locust Team,
I've been using locust for a while now, and really like to work with it. Still we achieved to run into some shortcomings for the project I am currently working on.
With this PR I would like to propose a new feature:
The possibility to create custom LoadTestShapes, where a step / tick only creates a specified set of users.
Why I need this feature / other maybe also need it:
In our Testscenario we need to make sure that a certain amount of locust users is definitely present upfront. Trying to achieve this with
weighting
orfixed_users
is possible but from my experience also mixed with a bit of luck. As we are already usingLoadTestShapes
I asked myself, whether it is possible to include the users in the stages for example.How to use it:
Using this would look like this for example:
In this case the first step would only create users of the class
WebsiteUserA
, the second stepWebsiteUserB
and so on.I also included a new example called
staging_user_classes.py
.Is it backwards compatible:
Yeah, it should be. If the
"user_classes"
-entry is missing, it will use all the existing user classes as input, as it was before.How it was achieved:
I needed to adjust three files:
shape.py
runners.py
dispatch.py
The maybe most important change (and maybe there's an even better solution for that) was, that in every
new_dispatch()
call (this happens for each tick / step once), also a newuser_generator
is being created. But in the end there were only a few minimal changes to the code done.I really am looking forward hearing your thought.
Thanks a lot in advance.