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

Screenshot feature removed in 4.0.4 #306

Closed
adamish opened this issue Oct 1, 2016 · 13 comments
Closed

Screenshot feature removed in 4.0.4 #306

adamish opened this issue Oct 1, 2016 · 13 comments
Projects

Comments

@adamish
Copy link

adamish commented Oct 1, 2016

The screenshot on failure functionality in 3.x appears to have been removed in 4.0.4.

Could this be restored?

@hastebrot
Copy link
Member

This is planned for the next release.

On Oct 1, 2016 5:45 PM, "Adam" notifications@github.com wrote:

The screenshot on failure functionality in 3.x appears to have been
removed in 4.0.4.

Could this be restored?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#306, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAeSf2YoZZDPUibZ49YpglS3qBHQstLmks5qvoALgaJpZM4KLyMs
.

@brcolow
Copy link
Collaborator

brcolow commented Jan 31, 2017

I am not sure but I think that to implement this feature for the junit5 provider we will need junit-team/junit5#618 to be resolved. I will work on adding this functionality to the junit4 provider either in #355 where the capture functionality was added or I will extract out the capture functionality from 355.

@brcolow
Copy link
Collaborator

brcolow commented Mar 14, 2017

I think that screenshot capture should be moved out of testfx-core and into the junit sub-projects. This way we can get better naming for the saved images and even report the failures with their respective screenshots by utilizing reporter functionality. As I stated I believe junit5 is not quite yet ready to support this (however I could be wrong about that and if so, yay) but junit4 can certainly support this with its' TestWatcher interface. I will work on this and open a PR very soon.

@hastebrot
Copy link
Member

screenshot capture should be moved out of testfx-core and into the junit sub-projects

Sounds good to me. 😃

@raniejade
Copy link
Contributor

Is there a plan to re-enable this?

private static <T> void verifyThatImpl(String reason,
                                           T value,
                                           Matcher<? super T> matcher) {
        try {
            MatcherAssert.assertThat(reason, value, matcher);
        }
        catch (AssertionError error) {
            // TODO: make error capture and assertion throw more reliable.
            // captureErrorScreenshot();
            throw new AssertionError(error.getMessage());
        }
    }

@JordanMartinez
Copy link
Contributor

JordanMartinez commented Aug 25, 2017

@brcolow Should this feature be re-added to DebugUtils? One could execute the captureErrorScreenshot() using DebugUtils#runCode, correct? If we added a convenience method (much like informedErrorHandler()), each test framework (JUnit 4/5, Spock) could further specify where it is saved and the name of the screenshot.

@brcolow
Copy link
Collaborator

brcolow commented Aug 25, 2017

Can we add the auto-capture behavior to informedErrorHandler()? It could print out the location of the screenshot (not super useful for CI without further infrastructure in place such as uploading failure screenshots to some s3 bucket or something) but definitely useful when running the our test suite (or developers using TestFX to run their tests locally) locally.

@adamish
Copy link
Author

adamish commented Aug 25, 2017

I ended writing a simple JUnit @rule that catches any exception and takes screenshot. I found this more useful as my tests involve fx matchers, various junit/hamcrest asserts and mockito. This approach works in all cases...

@brcolow
Copy link
Collaborator

brcolow commented Aug 25, 2017

@adamish IMO it makes sense for TestFX verifyThat calls to take and save a screenshot as TestFX is for testing GUI behavior. In many cases exceptions could be thrown from non-UI related infrastructure in a UI test (and could fail a junit/hamcrest assertion) and taking a screenshot in this case would provide no useful information. We should make the screenshot feature of verifyThat as easy as possible to toggle (should it default to off?) on or off, but I think it has a very logical place as part of the TestFX library functionality. That being said, I am glad you found a workaround/solution to the problem, and sorry it is taking so long to re-implement this. Going to leave the issue open until the functionality has been restored.

@JordanMartinez
Copy link
Contributor

I agree with @brcolow. Although one could use a Rule to accomplish this, the code I've written in #422 also allows a developer to take a screenshot of the test even when it doesn't fail without needing to get deep into TestFX's still undocumented API. I'm not sure why one would want to do that, but you never know.

@JordanMartinez
Copy link
Contributor

@brcolow Do you close issues immediately after they've been fixed/resolved (like this one) or only after there is a new release with the fix?

@brcolow
Copy link
Collaborator

brcolow commented Aug 27, 2017

I haven't really came up with a policy TBH. I think closing as soon as they are fixed/resolved makes sense - most of the time I just forget.

@brcolow brcolow closed this as completed Aug 27, 2017
@JordanMartinez
Copy link
Contributor

Thanks for the clarification. Yeah... the admin work behind open source projects can be a pain...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Robots
Backlog
Development

No branches or pull requests

5 participants