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

Added interactFast(...) Method to FxRobot(Interface) #268

Merged
merged 1 commit into from Dec 2, 2016

Conversation

Ortner
Copy link
Contributor

@Ortner Ortner commented May 5, 2016

The interactFast method doesn't wait for the gui pulse which makes it
much faster. It is especially useful for timing relevant tests e.g.
animations and for reading parameters of the gui thread.

@hastebrot
Copy link
Member

hastebrot commented May 9, 2016

Thanks for the PR.

Yes, we definitely need this method.

I'm not sure if interactFast() is the right name. I'd consider interactNoWait(), because ...NoWait() is internally used for methods that don't use waitForFxEvents(). However this could confuse the meaning, since the unrelated waitFor(asyncFx(...)) is used in this method.

@Ortner
Copy link
Contributor Author

Ortner commented May 10, 2016

I guess with the Callable it's not a problem (as the result is returned), but in the combination with the Runnable as parameter, I thought that the first impression would be that we do not wait for the Runnable to execute. I assume no one would think of a interact method just using Platform.runLater(...), so less waiting wouldn't make sense in this method. The javadoc explicitly mentions that it will be waited for the execution of the Runnable/Callable. So NoWait() should be OK.

I'm going to change the name.

Ortner pushed a commit to Ortner/TestFX that referenced this pull request May 10, 2016
@Ortner
Copy link
Contributor Author

Ortner commented May 10, 2016

By the way, any tests required for integration? If so, could you point me to the file where to implement it?

public FxRobot interactNoWait(Runnable runnable) {
waitFor(asyncFx(runnable));
return this;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing space between the two new methods.

@@ -187,8 +187,45 @@ public PointQuery offset(String query,
// METHODS FOR INTERACTION AND INTERRUPTION.
//---------------------------------------------------------------------------------------------

/**
* Calls a runnable on the FX application thread and waits for it and consecutive events to
Copy link
Collaborator

@brcolow brcolow Dec 1, 2016

Choose a reason for hiding this comment

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

The JavaDoc comment leading stars look misaligned, they should be aligned like so:

/**
 * Blah blah.
 *
 * @param blah
 * @return blah
 */

* @param runnable the runnable
* @return this robot
*/
public FxRobotInterface interactNoWait(Runnable runnable);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing space between new methods.

@brcolow
Copy link
Collaborator

brcolow commented Dec 1, 2016

I restarted the Travis CI build so that you can see the lint errors. I know its been a long time, but are you still interested in moving forward with this PR? Let me know, thanks very much @Ortner .

@Ortner
Copy link
Contributor Author

Ortner commented Dec 2, 2016

All done.

@brcolow
Copy link
Collaborator

brcolow commented Dec 2, 2016

Thanks very much, could you squash the 4 commits into one with an updated commit message? I will then merge immediately.

@Ortner
Copy link
Contributor Author

Ortner commented Dec 2, 2016

I can try (but need to find a HowTo first :-).

@brcolow
Copy link
Collaborator

brcolow commented Dec 2, 2016

git rebase -i HEAD~4
# The editor you have configured will popup
# Inside that editor, there will be 4 commits all proceeded by the word 'pick'
# Change the bottom 3 picks to `fixups` and the first pick to `reword`
# Write the commit message accordingly, and save and quit your editor (e.g. `:wq` in vim)
git push --force origin dev2

You can set your git text editor thusly:

git config --global core.editor emacs (or vim, or nano)

The interactNoWait method doesn't wait for the gui pulse which makes it
much faster. It is especially useful for timing relevant tests e.g.
animations and for reading parameters of the gui thread.
@Ortner
Copy link
Contributor Author

Ortner commented Dec 2, 2016

Guess it worked. I left out the merge of the master branch, as I assume I should not merge those in the commit.

@brcolow brcolow merged commit 68a87fe into TestFX:master Dec 2, 2016
@brcolow
Copy link
Collaborator

brcolow commented Dec 2, 2016

Merged. Thanks a lot for your patience.

@Ortner
Copy link
Contributor Author

Ortner commented Dec 3, 2016

Thanks for integrating!

@Ortner Ortner deleted the dev2 branch December 3, 2016 11:33
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

Successfully merging this pull request may close these issues.

None yet

3 participants