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

Update shape class' runner when Web UI picker is used #2534

Merged

Conversation

wiatrak2
Copy link
Contributor

@wiatrak2 wiatrak2 commented Jan 4, 2024

a shape class' .runner member is set by _create_runner method of Environment class. However, when the UI picker is used, the shape_class member of an Environment instance is not initialized yet, hence, there's no Runner assignment on initialization. Therefore, if you'd like to refer to self.runner in your custom test shape (for instance to get the number of users running), you receive an error saying that self.runner is None.

Hence, I added the shape class runner initialization also when the shape class is selected with UI picker.

@cyberw
Copy link
Collaborator

cyberw commented Jan 4, 2024

Can you add a test case that fails without this change?

@wiatrak2
Copy link
Contributor Author

wiatrak2 commented Jan 4, 2024

There you go :)

@cyberw
Copy link
Collaborator

cyberw commented Jan 5, 2024

I see the need for the change, but update_shape_class_runner is a very weird method. Perhaps it can be refactored/renamed to something that makes more sense. Or just remove the method and have the "weird" behavior inline, where it is at least readable.

@wiatrak2
Copy link
Contributor Author

wiatrak2 commented Jan 5, 2024

I believe that we should avoid code repetition. Also I think that the action should be encapsulated by a method, as it wouldn't be clean to modify Environment's members from inside the WebUI. Thinking about a better name, my candidate is set_runner_in_shape_class - what do you think? 😅

@cyberw
Copy link
Collaborator

cyberw commented Jan 5, 2024

How about doing it here: https://github.com/locustio/locust/pull/2534/files#diff-498b8c363e788cc0210ac8600bce7660c60d210cad41af086886b79055c717beR636
, where we are already accessing the shape class directly.

Something like
shape_class.runner = self.environment.runner

environment-runner separation is very messy, and while I dont like duplication, I like methods with unclear meaning even less :)

@wiatrak2
Copy link
Contributor Author

wiatrak2 commented Jan 5, 2024

your code, your rules ;)

@cyberw cyberw merged commit d557d1e into locustio:master Jan 5, 2024
15 checks passed
@cyberw
Copy link
Collaborator

cyberw commented Jan 5, 2024

Thx!

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.

None yet

2 participants