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

Remove legacy UI #2703

Merged
merged 5 commits into from
May 7, 2024
Merged

Remove legacy UI #2703

merged 5 commits into from
May 7, 2024

Conversation

andrewbaldwin44
Copy link
Collaborator

@andrewbaldwin44 andrewbaldwin44 commented May 6, 2024

  • Remove words "legacy" and "modern" from codebase as we only have one UI now
  • Remove legacy-ui and modern_ui flags
  • Remove logic for rendering legacy UI
  • Remove templates and static assets for legacy UI
  • Update UI tests to suit the modern UI
  • Update documentation to only include logic for modern UI
  • Ensured modern UI, web login, HTML report, and downloaded HTML report all continue to work as expected

@andrewbaldwin44 andrewbaldwin44 force-pushed the task/2673 branch 5 times, most recently from 1778381 to f9fe009 Compare May 6, 2024 00:55
@cyberw
Copy link
Collaborator

cyberw commented May 6, 2024

Nice!

Can we keep the argument around (and hide it using help=configargparse.SUPPRESS), and exit with an error message if someone sets it?

@andrewbaldwin44 andrewbaldwin44 force-pushed the task/2673 branch 3 times, most recently from 2e7577d to 2294b2f Compare May 6, 2024 14:03
locust/main.py Outdated
@@ -183,7 +179,8 @@ def is_valid_percentile(parameter):
options.spawn_rate = options.hatch_rate

if options.legacy_ui:
sys.stderr.write("[DEPRECATED] The legacy UI is deprecated and will be removed soon\n")
sys.stderr.write("[DEPRECATED] The legacy UI is deprecated")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think "deprecated" usually means "going to be deleted, but still works", so this line is confusing :)

)
self.assertRegex(response.text, re_spawn_rate)
self.assertNotRegex(response.text, re_disabled_spawn_rate)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These regex tests are being removed because they are covered by our frontend tests

self.assertEqual(
self.environment.available_user_classes["User1"].json(),
{"host": "http://localhost", "tasks": ["my_task_2"], "fixed_count": 0, "weight": 1},
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These tests were just moved to join the base Web UI tests

@cyberw
Copy link
Collaborator

cyberw commented May 6, 2024

LGTM. Especially if you add help=configargparse.SUPPRESS in argument_parser.py :)

I'll want to hold off a day or two on merging though, because I'd like to release the changes already merged as a separate release.

@cyberw
Copy link
Collaborator

cyberw commented May 7, 2024

Nice!

@cyberw cyberw merged commit 4be0a7e into locustio:master May 7, 2024
14 checks passed
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.

2 participants