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

Use Modern UI by default #2569

Merged
merged 2 commits into from
Jan 29, 2024

Conversation

andrewbaldwin44
Copy link
Collaborator

Changes implemented

  • Updated screenshots to be screenshots of the new UI
  • Changed the --modern-ui flag to be --legacy-ui and updated the relevant tests to use this flag
  • Updated the docs

@cyberw
Copy link
Collaborator

cyberw commented Jan 29, 2024

Nice! Why are some of the tests run with legacy UI though? Ideally they should work in both, of course, but it is most important that they work in modern-ui :)

@andrewbaldwin44
Copy link
Collaborator Author

Yeah it's because they are testing things using the legacy UI templates or the legacy UI HTML template. Even though we are not developing the legacy UI anymore, I figured we should keep them to ensure new developments to the modern UI don't cause regressions

@cyberw
Copy link
Collaborator

cyberw commented Jan 29, 2024

Yea, testing some things with the old UI totally makes sense. But why wouldnt we want to test --autostart with modern UI?

@andrewbaldwin44
Copy link
Collaborator Author

Oh the --autostart flag is testing the modern UI, it uses the modern UI by default now :) if it were not using the modern UI it would need the --legacy-ui flag

@cyberw
Copy link
Collaborator

cyberw commented Jan 29, 2024

But test_autostart_wo_run_time is only run against legacy UI? Or am I misreading the diff somehow?

@andrewbaldwin44
Copy link
Collaborator Author

Yeah you're right, but I believe it's because of this line: self.assertIn('<body class="running">', response.text), which would fail in the modern UI. If you prefer I could change this so it just tests the modern UI, or I can add an additional test so it tests the modern UI as well

@cyberw
Copy link
Collaborator

cyberw commented Jan 29, 2024

Hows about you update the test so that it works with modern UI but add a ”legacy” test (based this one or one of the more basic ones) that run with legacy ui?

@andrewbaldwin44 andrewbaldwin44 force-pushed the feature/modern-ui-default branch from 31aebde to 641ed7d Compare January 29, 2024 21:26
@andrewbaldwin44
Copy link
Collaborator Author

Ok I updated the autorun tests. They don't run against the legacy UI anymore, but I think these should be sufficient since we're not really testing the UI, but rather the output of the template arguments to ensure the state is set as "running"

@cyberw cyberw merged commit f712638 into locustio:master Jan 29, 2024
15 checks passed
@cyberw cyberw changed the title Modern UI as Default Use Modern UI by default Feb 4, 2024
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