locustio
fix: update `parsed_options` `run_time`
Description
Hit a bug where, by using a custom LoadTestShape with use_common_options (as described here) noticed that the run_time set through the web UI is not persisted to parsed_options (which I'd be accessing from my custom shape class' self.runner.environment.parsed_options.run_time). This value was thus always grabbing the initial value set through config / cli options, which is pretty unexpected to me. I understand the intended use case here, as specified in the docs, is to "reuse command line parameters", but since the web UI still allows changing this value updates should IMO reflect everywhere they're referenced downstream.
Changes
Persist this value to parsed_options_dict on the swarm endpoint handler. It'd make even more sense to me to do this for all values though, by changing the elif condition on current line 235 to if, not sure why it's done that way and why not all values are persisted/propagated.
Thanks!
Does changing it from elif to if break any of the tests? If not, then I think that is preferable, like you say. Theres's probably some historic reason (not wanting to risk changing something when we introduced the ability to set extra web ui params)
When changing elif -> if on L235 two tests fail:
======================================== short test summary info ========================================
FAILED locust/test/test_web.py::TestWebUI::test_swarm_updates_parsed_options_when_multiple_userclasses_specified - AssertionError: Lists differ: ['U', 's', 'e', 'r', '1'] != ['User1', 'User2']
FAILED locust/test/test_web.py::TestWebUI::test_swarm_updates_parsed_options_when_single_userclass_specified - AssertionError: Lists differ: ['U', 's', 'e', 'r', '1'] != ['User1']
luckily written on https://github.com/locustio/locust/pull/2201. And it makes sense actually, as would have to handle the parsing of each option (run_time for instance that I'm interested in needs parse_timespan). So it needs a bigger change. I also think these options should be exposed on a different place than environment.parsed_options, i.e. leave these read-only, and have another attribute with actual runtime values at any moment easily accessible on environment. I'm just getting started with the lib so not feeling confident enough about any of this though 😄
As for my PR as is, well, noticed that the same issue exists for parsed_optionss .num_users and .spawn_rate, so... not sure on the value of my single change, it even makes it more confusing updating only a few values.
I think it is only user_classes that needs special treatment. What if you do it similar to this:
if key == "user_classes": # need special treatment because it isnt a plain value
parsed_options_dict[key] = request.form.getlist("user_classes")
elif key in parsed_options_dict:
parsed_options_value = parsed_options_dict[key]
...
I'm quite interested in a solution for this too... In our environment, not everyone has access to touch the locust code and rerun with new params, but they have access to the UI and we want them to use our custom shapes. Cheers!
