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

new prefix for images produced in testing: baseline_ or thisPR_ #4956

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

nastasha-w
Copy link
Contributor

PR Summary

I'm adding prefixes to the file names produced in yt/yt/utilities/answer_testing/framework.py, dump_images function. This should make it straightforward to see which image is the baseline for the comparison and which has been produced by the pull request branch in the nose testing output.

This addresses issue #4954.

Copy link
Member

@neutrinoceros neutrinoceros left a comment

Choose a reason for hiding this comment

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

@neutrinoceros
Copy link
Member

You can't just revert these commits by stacking more commits, these files need to be obliterated from the history.

@nastasha-w
Copy link
Contributor Author

@neutrinoceros I think I've fixed it for this specific branch; please let me know if it's not ok.

@neutrinoceros
Copy link
Member

Perfect !

@neutrinoceros neutrinoceros added enhancement Making something better tests: running tests Issues with the test setup labels Jul 24, 2024
@neutrinoceros neutrinoceros added this to the 4.4.0 milestone Jul 24, 2024
@neutrinoceros
Copy link
Member

Before we merge this, it'd be great to check if it actually works as we think it does. Can you alter some image answer tests so we get to see a diff report ? We'll remove the temporary commit afterwards

@nastasha-w
Copy link
Contributor Author

nastasha-w commented Jul 24, 2024

I was hoping the tests on PR #4939 would show that, since those image tests are failing anyway. It's why I merged these changes into that branch.

@neutrinoceros
Copy link
Member

I believe you mean #4939

@nastasha-w
Copy link
Contributor Author

I believe you mean #4939

Yup, thanks! I edited it.

@nastasha-w
Copy link
Contributor Author

It seems to work! The file names in the latest tests for PR #4939 (link) are e.g.
'baseline_jkwqzihf.png'
'thisPR_4d5dlaqe.png'
'thisPR_4d5dlaqe-failed-diff.png'

@neutrinoceros neutrinoceros merged commit 2461af6 into yt-project:main Jul 25, 2024
13 checks passed
Copy link

welcome bot commented Jul 25, 2024

Hooray! Congratulations on your first merged pull request! We hope we keep seeing you around! 🎆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Making something better tests: running tests Issues with the test setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants