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

Make KeyboardShortcutsConfigTest run reliably on Travis #1263

Closed
dariusf opened this issue Feb 16, 2016 · 9 comments · Fixed by #1368
Closed

Make KeyboardShortcutsConfigTest run reliably on Travis #1263

dariusf opened this issue Feb 16, 2016 · 9 comments · Fixed by #1368

Comments

@dariusf
Copy link
Contributor

dariusf commented Feb 16, 2016

guitests.KeyboardShortcutsConfigTest > invalidKeySpecified FAILED
    org.loadui.testfx.exceptions.NoNodesVisibleException: Matching nodes were found, but none of them were visible. Screenshot saved as /home/travis/build/HubTurbo/HubTurbo/screenshot1455554274261.png.
        at org.loadui.testfx.GuiTest.getVisibleNodes(GuiTest.java:238)
        at org.loadui.testfx.GuiTest.find(GuiTest.java:466)
        at org.loadui.testfx.GuiTest.find(GuiTest.java:306)
        at org.loadui.testfx.GuiTest.pointFor(GuiTest.java:1159)
        at org.loadui.testfx.GuiTest.move(GuiTest.java:849)
        at org.loadui.testfx.GuiTest.click(GuiTest.java:627)
        at guitests.KeyboardShortcutsConfigTest.invalidKeySpecified(KeyboardShortcutsConfigTest.java:67)
guitests.KeyboardShortcutsConfigTest > invalidKeySpecified FAILED
    java.lang.RuntimeException: java.lang.NullPointerException
        at guitests.UITest.awaitCondition(UITest.java:355)
        at guitests.UITest.awaitCondition(UITest.java:341)
        at guitests.UITest.waitUntilNodeAppears(UITest.java:266)
        at guitests.KeyboardShortcutsConfigTest.invalidKeySpecified(KeyboardShortcutsConfigTest.java:65)
        Caused by:
        java.lang.NullPointerException
            at com.sun.javafx.text.PrismTextLayout.addTextRun(PrismTextLayout.java:755)
            at com.sun.javafx.text.GlyphLayout.breakRuns(GlyphLayout.java:322)
            at com.sun.javafx.text.PrismTextLayout.buildRuns(PrismTextLayout.java:770)
            at com.sun.javafx.text.PrismTextLayout.layout(PrismTextLayout.java:1021)
            at com.sun.javafx.text.PrismTextLayout.ensureLayout(PrismTextLayout.java:223)
            at com.sun.javafx.text.PrismTextLayout.getBounds(PrismTextLayout.java:246)
            at javafx.scene.text.Text.getLogicalBounds(Text.java:358)
            at javafx.scene.text.Text.impl_computeGeomBounds(Text.java:1168)
            at javafx.scene.Node.updateGeomBounds(Node.java:3577)
            at javafx.scene.Node.getGeomBounds(Node.java:3530)
            at javafx.scene.Node.getLocalBounds(Node.java:3478)
            at javafx.scene.Node.updateTxBounds(Node.java:3641)
            at javafx.scene.Node.getTransformedBounds(Node.java:3424)
            at javafx.scene.Parent.getChildTransformedBounds(Parent.java:1732)
            at javafx.scene.Parent.recomputeBounds(Parent.java:1524)
            at javafx.scene.Parent.impl_computeGeomBounds(Parent.java:1388)
            at javafx.scene.layout.Region.impl_computeGeomBounds(Region.java:3078)
            at javafx.scene.Node.updateGeomBounds(Node.java:3577)
            at javafx.scene.Node.getGeomBounds(Node.java:3530)
            at javafx.scene.Node.getLocalBounds(Node.java:3478)
            at javafx.scene.Node$MiscProperties$2.computeBounds(Node.java:6472)
            at javafx.scene.Node$LazyBoundsProperty.get(Node.java:9306)
            at javafx.scene.Node$LazyBoundsProperty.get(Node.java:9276)
            at javafx.scene.Node.getBoundsInLocal(Node.java:3156)
            at org.loadui.testfx.utils.FXTestUtils.isNodeWithinSceneBounds(FXTestUtils.java:248)
            at org.loadui.testfx.utils.FXTestUtils.isNodeVisible(FXTestUtils.java:242)
            at org.loadui.testfx.utils.FXTestUtils$6.apply(FXTestUtils.java:233)
            at org.loadui.testfx.utils.FXTestUtils$6.apply(FXTestUtils.java:230)
            at com.google.common.collect.Iterators.indexOf(Iterators.java:778)
            at com.google.common.collect.Iterators.any(Iterators.java:684)
            at com.google.common.collect.Iterables.any(Iterables.java:623)
            at com.google.common.collect.Collections2$FilteredCollection.isEmpty(Collections2.java:186)
            at org.loadui.testfx.GuiTest.getVisibleNodes(GuiTest.java:237)
            at org.loadui.testfx.GuiTest.find(GuiTest.java:466)
            at guitests.UITest.findQuiet(UITest.java:293)
            at guitests.UITest.lambda$waitUntilNodeAppears$8(UITest.java:266)
            at guitests.UITest.awaitCondition(UITest.java:347)
            ... 3 more
@tsoonjin
Copy link
Contributor

Is it ok if I work on this ?

@tsoonjin
Copy link
Contributor

@dariusf any specific steps to reproduce the issue. Perhaps running together with other guitests because running only the test alone seems to have no problem. Maybe I will try a few more times.

@dariusf
Copy link
Contributor Author

dariusf commented Feb 17, 2016

Unfortunately, no, the only thing we have are those stack traces, so the best way might be to reason the problem out. This rarely fails, and passes quite reliably on OS X/my Ubuntu VM. Interactions with other tests may be the cause, since this does rely on global state which other tests can access.

@hastebrot
Copy link

So, the NullPointerException and NoNodesVisibleException are caused by a line in invalidKeySpecified():

@Test
public void invalidKeySpecified() {
    // ...
    waitUntilNodeAppears(hasText("Invalid key specified for MARK_AS_READ" +
        " or it has already been used for some other shortcut. "));
    // ...
}

Which uses a test helper method from UITest:

public void waitUntilNodeAppears(Node node) {
    waitUntil(node, n -> n.isVisible() && n.getParent() != null);
}

Which in turn uses waitUntil from TestFX 3.1.0, with a timeout of 15 seconds.

public static <T extends Node> void waitUntil( final T node, final Predicate<T> condition )
{
    waitUntil( node, condition, 15 );
}

public static <T extends Node> void waitUntil( final T node, final Predicate<T> condition, int timeoutInSeconds )
{
    TestUtils.awaitCondition( new Callable<Boolean>()
    {
        @Override
        public Boolean call() throws Exception
        {
            return condition.apply( node );
        }
    }, timeoutInSeconds );
}

public class TestUtils
{
    public static void awaitCondition( Callable<Boolean> condition )
    {
        awaitCondition( condition, 5 );
    }

    public static void awaitCondition( Callable<Boolean> condition, int timeoutInSeconds )
    {
        long timeout = System.currentTimeMillis() + timeoutInSeconds * 1000;
        try
        {
            while( !condition.call() )
            {
                Thread.sleep( 10 );
                if( System.currentTimeMillis() > timeout )
                {
                    throw new TimeoutException();
                }
            }
        }
        catch( Exception e )
        {
            throw new RuntimeException( e );
        }
    }
}

@hastebrot
Copy link

So hasText() couldn't find the Node with text/label "Invalid key specified for MARK_AS_READ...", and after 15 seconds waitUntilNodeAppears() wraps the NPE (caused by n.isVisible()) into a RuntimeException a throws it.

@hastebrot
Copy link

Now, I wonder why the stacktrace says that the NPE is thrown in KeyboardShortcutsConfigTest.java:65.

@hastebrot
Copy link

hasText() is also a bit problematic, it uses GuiTest.find() which in turn uses assertNodesFound() and getVisibleNodes(). So, the condition with n.isVisible() is a bit redundant here.

@hastebrot
Copy link

@dariusf You said it might be a problem with global state. Does this mean it is unlikely to be caused by a timing problem?

@dariusf
Copy link
Contributor Author

dariusf commented Mar 1, 2016

I'm not entirely sure, but the latter seems more likely. Will take a closer look to confirm.

The overload involved there seems to be this one instead:

public void waitUntilNodeAppears(Matcher<Object> matcher) {
    awaitCondition(() -> findQuiet(matcher).isPresent());
}

private <T extends Node> Optional<T> findQuiet(Matcher<Object> matcher) {
    try {
        return Optional.of(find(matcher));
    } catch (NoNodesFoundException | NoNodesVisibleException e) {
        return Optional.empty();
    }
}

which ends up doing Optional.of(null). I think that's the cause of the NPE. The rest of it propagating up through awaitCondition is as you said.

The differing line numbers should be from an old version of the file. There are other parts which don't match up exactly also.

If, say, this were a timing problem, is busy-waiting like this a good way to fix it? I'm afraid we're still a bit unclear on best practices for synchronisation. 😛

Thanks for helping us with this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants