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

[WIP] Unit Test for dashboard #672

Closed
wants to merge 22 commits into from
Closed

Conversation

proy30
Copy link
Contributor

@proy30 proy30 commented Aug 11, 2024

This PR introduces the first unit test for the ImpactX dashboard, leveraging the SeleniumBase library to simulate a simulation run by a user with the GUI.

Key Features

  • The unit test runs a simulation using the parameters from the FODO cell example on the GUI. (takes ~1 minute)
  • The test is considered successful if the simulation is completed without errors.

Issues

  • Dependency on Running Dashboard:
    • The current implementation requires the ImpactX dashboard to run in a separate terminal before executing the test via pytest.
    • Can investigate to potentially have dashboard be launched within the test itself.
    • Unit test is flaky with passing/failing, unsure if this is due to code error or error in pytest implementation. Need to investigate. Started occurring after the implementation of subprocess.
  • Adding seleniumbase as a dependency
    • Unsure where to add. (implemented incorrectly)

Checklist

  • Implement unit test for impactx-dashboard (testing with FODO cell simulation parameters.
  • Automate the dashboard launch within the test.
  • Fix flaky unit test
  • Include seleniumbase dependency in CI test files
  • Include trame dependencies in CI test files

Resolves #667

Contributes to unit testing by applying a UI component to exit the dialog box and allowing selenium to continue testing the application
Initially these were added to convert user input to the correct type in order to be compatible with simulation. However, upon commenting these out for the sake of unit testing compatibility, it was found that the simulation is still functional with user inputs and does not need any conversion of user values. There is potential for underlying problems that have yet to be found.
@proy30 proy30 added the component: dashboard our browser based trame dashboard label Aug 11, 2024
@proy30 proy30 self-assigned this Aug 11, 2024
@proy30 proy30 requested a review from ax3l August 11, 2024 04:53
.github/workflows/stubs.yml Outdated Show resolved Hide resolved
Comment on lines +31 to +33
script_dir = os.path.dirname(os.path.abspath(__file__))
working_directory = os.path.join(script_dir, "../../../src/python/impactx")
working_directory = os.path.normpath(working_directory)
Copy link
Member

Choose a reason for hiding this comment

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

We need to generalize this so it:

  • runs in CTests from the build directory
  • runs when copied in a package manager with external installs

working_directory = os.path.normpath(working_directory)

return subprocess.Popen(
["python", "-m", "dashboard"],
Copy link
Member

Choose a reason for hiding this comment

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

When I run this locally, this literally opens a browser window for me.
Is there a way to run this headless?

Mark the test as skipped if `seleniumbase` is not available.
.github/workflows/stubs.yml Outdated Show resolved Hide resolved
Comment on lines 66 to 68
# Check if "Simulation complete" message is printed
xterm_content = sb.get_text("#xterm_component")
assert "Simulation complete." in xterm_content
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is currently buggy and offering flaky results.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternative method implemented by having unit test interact with phase space projections, which indirectly mean simulation ran properly.

time.sleep(10)

try:
with SB() as sb:
Copy link
Member

Choose a reason for hiding this comment

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

Trying if this prevents that the unit test on Linux/macOS opens a browser:

Suggested change
with SB() as sb:
with SB(headless=True) as sb:

Copy link
Member

Choose a reason for hiding this comment

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

This indirectly assures user that sim ran successfully.
@ax3l
Copy link
Member

ax3l commented Sep 19, 2024

Continued in #706

@ax3l ax3l closed this Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: dashboard our browser based trame dashboard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dashboard] Missing Unit Testing
2 participants