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 issue where empty WebUI property is not parsed correctly #3012

Merged
merged 1 commit into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 56 additions & 4 deletions locust/test/test_web.py
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ def tick(self):
response = requests.get("http://127.0.0.1:%i/stop" % self.web_port)
self.assertEqual(response.json()["message"], "Test stopped")

def test_swarm_custom_argument(self):
def test_swarm_custom_argument_without_default_value(self):
my_dict = {}

class MyUser(User):
Expand All @@ -776,10 +776,10 @@ def my_task(self):
my_dict["val"] = self.environment.parsed_options.my_argument

@locust.events.init_command_line_parser.add_listener
def _(parser, **kw):
def _(parser):
parser.add_argument("--my-argument", type=int, help="Give me a number")

parsed_options = parse_options(args=["--my-argument", "42"])
parsed_options = parse_options()
self.environment.user_classes = [MyUser]
self.environment.parsed_options = parsed_options
self.environment.web_ui.parsed_options = parsed_options
Expand All @@ -788,7 +788,59 @@ def _(parser, **kw):
data={"user_count": 1, "spawn_rate": 1, "host": "", "my_argument": "42"},
)
self.assertEqual(200, response.status_code)
self.assertEqual(my_dict["val"], 42)
self.assertEqual("42", my_dict["val"])

def test_swarm_custom_argument_with_default_value(self):
my_dict = {}

class MyUser(User):
host = "http://example.com"
wait_time = constant(1)

@task(1)
def my_task(self):
my_dict["val"] = self.environment.parsed_options.my_argument

@locust.events.init_command_line_parser.add_listener
def _(parser):
parser.add_argument("--my-argument", type=int, help="Give me a number", default=24)

parsed_options = parse_options()
self.environment.user_classes = [MyUser]
self.environment.parsed_options = parsed_options
self.environment.web_ui.parsed_options = parsed_options
response = requests.post(
"http://127.0.0.1:%i/swarm" % self.web_port,
data={"user_count": 1, "spawn_rate": 1, "host": "", "my_argument": "42"},
)
self.assertEqual(200, response.status_code)
self.assertEqual(42, my_dict["val"])

def test_swarm_override_command_line_argument(self):
my_dict = {}

class MyUser(User):
host = "http://example.com"
wait_time = constant(1)

@task(1)
def my_task(self):
my_dict["val"] = self.environment.parsed_options.my_argument

@locust.events.init_command_line_parser.add_listener
def _(parser):
parser.add_argument("--my-argument", type=int, help="Give me a number")

parsed_options = parse_options(args=["--my-argument", "24"])
self.environment.user_classes = [MyUser]
self.environment.parsed_options = parsed_options
self.environment.web_ui.parsed_options = parsed_options
response = requests.post(
"http://127.0.0.1:%i/swarm" % self.web_port,
data={"user_count": 1, "spawn_rate": 1, "host": "", "my_argument": "42"},
)
self.assertEqual(200, response.status_code)
self.assertEqual(42, my_dict["val"])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks for adding tests!


def test_swarm_host_value_not_specified(self):
class MyUser(User):
Expand Down
4 changes: 2 additions & 2 deletions locust/web.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,9 @@ def swarm() -> Response:
if isinstance(parsed_options_value, bool):
parsed_options_dict[key] = value == "true"
elif parsed_options_value is None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can do:

elif parsed_options_value is None and value is None:
  parsed_options_dict[key] = None

This way we still get the type casting from the else statement

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the parsed_options_value is None, type checking does not make sense. I modified the code to catch this case.

Copy link
Collaborator

@andrewbaldwin44 andrewbaldwin44 Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type checking indeed doesn't make sense if the value is None, but take for example if you have an argument with a default value of None:

parser.add_argument('--number', default=None)

Here the parsed_options_value will always be None, however in some cases the value will be a number, in which case keeping the type may be useful

That's why I suggested elif parsed_options_value is None and value is None

parsed_options_dict[key] = parsed_options_value
parsed_options_dict[key] = value
else:
parsed_options_dict[key] = type(parsed_options_dict[key])(value)
parsed_options_dict[key] = type(parsed_options_value)(value)

if environment.shape_class and environment.runner is not None:
environment.runner.start_shape()
Expand Down
Loading