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

Save screenshot on App.run_test failure #3702

Closed
ab-10 opened this issue Nov 18, 2023 · 8 comments
Closed

Save screenshot on App.run_test failure #3702

ab-10 opened this issue Nov 18, 2023 · 8 comments

Comments

@ab-10
Copy link

ab-10 commented Nov 18, 2023

Suggestion to add an option screenshot_on_fail: Optional[Path|str] = None in App.run_test to save a screenshot of the app upon a test failure and error.
Happy to implement the feature myself, if it aligns with the project's goals.

Example functionality

async with app.run_test(screenshot_on_fail="logs/") as pilot:  
    # Test pressing the R key
    await pilot.press("r")  

    assert app.screen.styles.background == Color.parse("red")

If the assertion would fail, this would save a screenshot of the app at the time of the failure in logs/textualize-<timestamp>.svg

Open questions

  1. How to enable the user to configure the save location and filename for the tests? What would be reasonable defaults here?
  2. Do other contributors find such functionality desirable?
@Textualize Textualize deleted a comment from github-actions bot Dec 4, 2023
@willmcgugan
Copy link
Collaborator

Interesting idea. Can you describe the kind of tests you might want this feature for?

@davep @rodrigogiraoserrao thoughts?

@ab-10
Copy link
Author

ab-10 commented Dec 4, 2023 via email

@rodrigogiraoserrao
Copy link
Contributor

@davep @rodrigogiraoserrao thoughts?

This looks helpful to debug tests.
It's something that could even be turned off by default and we just turn on locally when a test is failing for some reason we don't understand.

@davep
Copy link
Contributor

davep commented Dec 5, 2023

I'm not opposed to the idea, although I'm struggling to see the utility in it; the couple of examples given above seem to be like things I wouldn't ascertain from a screenshot anyway (you can't see IDs in a screenshot, for example).

@rodrigogiraoserrao
Copy link
Contributor

I'm not opposed to the idea, although I'm struggling to see the utility in it; the couple of examples given above seem to be like things I wouldn't ascertain from a screenshot anyway (you can't see IDs in a screenshot, for example).

Maybe the example isn't great but I've found myself wishing for this feature in the past, fyi...
Maybe that's not enough to implement it, but I would've used it in the past.

@ab-10
Copy link
Author

ab-10 commented Dec 6, 2023

Thank you @willmcgugan, @rodrigogiraoserrao, and @davep for commnents. The feature is a bit more controversial than I initially expected. I will close this issue, especially since I expect the actual implementation to have a couple of subjective elements in it as well (e.g. how to configure the storage path for the images).

@ab-10 ab-10 closed this as completed Dec 6, 2023
Copy link

github-actions bot commented Dec 6, 2023

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

@ab-10 ab-10 closed this as not planned Won't fix, can't repro, duplicate, stale Dec 6, 2023
Copy link

github-actions bot commented Dec 6, 2023

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

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

No branches or pull requests

4 participants