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

e2e-test: add savePlot #5479

Merged
merged 8 commits into from
Nov 25, 2024
Merged

e2e-test: add savePlot #5479

merged 8 commits into from
Nov 25, 2024

Conversation

midleman
Copy link
Contributor

@midleman midleman commented Nov 23, 2024

Intent

The goal of this PR was to improve the test by removing a hardcoded wait that was previously required for the test to work and pass. I was able to make improvements to the test and it runs/passes but occasionally I kept running into a flake of RPC timeout on CI runs for the python plot test. Maybe that was the original issue? (The in-line comment didn't say). Adding the wait back in does seem to remove the flake, so maybe that was it after-all. Regardless, there are still improvements worth merging in.

Approach

A new method, savePlot, was introduced to encapsulate and enhance the logic for saving a plot to a selected file type. Additionally, some before/after hooks were cleaned up for better readability.

QA Notes

  • Confirmed plots test PASS on Windows.
  • I did not see an open issue for the RPC timeout? If we don't have one, I can add it and tag it to this test.

@midleman midleman marked this pull request as ready for review November 25, 2024 13:42
jonvanausdeln
jonvanausdeln previously approved these changes Nov 25, 2024
Copy link
Contributor

@jonvanausdeln jonvanausdeln left a comment

Choose a reason for hiding this comment

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

LGTM!

@midleman midleman merged commit c3c4896 into main Nov 25, 2024
4 checks passed
@midleman midleman deleted the mi/update-plots branch November 25, 2024 19:16
@github-actions github-actions bot locked and limited conversation to collaborators Nov 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants