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.

Sign up for GitHub

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

Jump to bottom

281: Vulnerability in WaitForAsyncUtils #351

Merged
merged 2 commits into from Jan 18, 2017
Merged

281: Vulnerability in WaitForAsyncUtils #351

merged 2 commits into from Jan 18, 2017

Conversation

brcolow
Copy link
Collaborator

@brcolow brcolow commented Jan 13, 2017

Test

@hastebrot
Copy link
Member

hastebrot commented Jan 13, 2017 •
edited

Removing @Test(timeout = 1000), is this a good idea?

You can also --amend git commits and push --force them. 😃

@brcolow
Copy link
Collaborator Author

brcolow commented Jan 13, 2017 •
edited

@hastebrot Indeed this is only a work-in-progress, not really ready to be seen.. I should have added a WIP tag (do we have a WIP tag?). Once the tests are passing I will squash to one commit :).

This PR is originally from Ortner, I am just trying to squash it down to one commit and change some stylistic aspects. Then I will investigate the logic behind it.

As to removing the timeout, I believe that it is still enforced with the global timeout specified via the @Rule, however as I stated I have not yet investigated the logic/reasoning behind this PR :).

@brcolow brcolow force-pushed the test-ortner branch 10 times, most recently from 4f4adf1 to f85f1be Compare January 14, 2017 06:43
@brcolow brcolow mentioned this pull request Jan 14, 2017
Closed
@brcolow brcolow force-pushed the test-ortner branch 3 times, most recently from e3505ba to cc3769a Compare January 15, 2017 01:53
@brcolow
Copy link
Collaborator Author

brcolow commented Jan 16, 2017

I am not sure how I feel about having two public static fields (printException and autoCheckException). It just seems like an awkward API but I am also not sure of a better solution.

Jan Ortner added 2 commits January 16, 2017 17:23
@brcolow
(refactor) Improve exception handling in WaitForAsyncUtils.
f1a3bf4 
Improve exception handling methods and allow the user to have more
control.

Fixed async_callable_with_exception to detect that no exception is
thrown.

Add testing for exceptions thrown on JavaFX application thread.

Closes TestFX#280
Closes TestFX#281
@brcolow
(fix) Failing tests in AWTRobotAdapterTest and GlassRobotAdapterTest.
a98bc65 
Waiting for all events in the FX-Eventqueue after setting up the stage
(FXToolKit.waitForSetup()). This ensures, that the GUI is ready for
test, after the stage is set up and control is handed over to the test
routines.

Closes TestFX#274
@brcolow
Copy link
Collaborator Author

brcolow commented Jan 18, 2017 •
edited

This is probably out of scope for this PR but I wonder if introducing slf4j as a dependency would make sense (I am thinking about this because this PR uses System.err.println to print exceptions). There are many scattered calls to System.out.println throughout the code base...but I wonder if these should be replaced with slf4j calls.

@brcolow brcolow merged commit 1f1956a into TestFX:master Jan 18, 2017
@brcolow brcolow mentioned this pull request Jan 18, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@brcolow @hastebrot