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

Add Windows build to CI #596

Merged
merged 8 commits into from
Oct 3, 2017
Merged

Add Windows build to CI #596

merged 8 commits into from
Oct 3, 2017

Conversation

JordanMartinez
Copy link
Contributor

No description provided.

@JordanMartinez
Copy link
Contributor Author

JordanMartinez commented Sep 30, 2017

It seems like AppVeyor is either not building the PR or taking a much longer time than normal to do so.

Edit: Turns out it was working on a previous build from a previous PR/commit that was outdated. Once I cancelled the old build, it started building the newer ones.

@JordanMartinez
Copy link
Contributor Author

Looks like the Windows build is getting stuck on NavigationTests.MultiLineJaggedTextTests.pressingDownMovesCaretToNextLine. My workaround for insuring the TextFlow was properly set up to have 3 lines seems to be the culprit.

@afester
Copy link
Collaborator

afester commented Oct 2, 2017

I have observed this earlier with some other tests running on Windows - the root cause is that
waitForMultiLineRegistration() NEVER returns, means it does not even time out after five seconds in WaitForAsyncUtils.waitFor(5, TimeUnit.SECONDS, textFlowIsReady);. A workaround I tried earlier was to explicitly terminate the while() loop, something like

int count = 0;
while (area.getParagraphLinesCount(0) != 3 && count++ < 500) {
   sleep(10);
}

With that , the test still fails, but at least it does not block forever.

@afester
Copy link
Collaborator

afester commented Oct 2, 2017

Why do we need that "MultiLineRegistration" workaround at all? Is it because of different font sizes/different layouts on different window systems (Mac vs. X11 vs. Windows)? During my earlier analysis, I also simply made the text longer so that it is spread over 3 lines, and the tests also passed ...

@JordanMartinez
Copy link
Contributor Author

JordanMartinez commented Oct 2, 2017

It never returns or times out? Very strange....

The workaround is needed because the tests fail on (corrected: not Linux, but) Mac. The tests would always fail because TextFlow wouldn't return the correct paragraph count immediately after it was set up. So, by waiting for the expected count to be the value returned, then the tests will work. Perhaps we should try running the test without this workaround to see which OS needs it and which do not.

@JordanMartinez
Copy link
Contributor Author

Hmm... seems it works even on Mac now without that method....
I wonder if splitting up the navigation tests into their respective classes also removed an interference between tests that I didn't know about.

@JordanMartinez
Copy link
Contributor Author

Since the font is never specified in the test, I'm guessing the tests pass on Linux/Mac because their default fonts are the same but that Windows uses a different default font. Thus, more/less content is being displayed per line than expected, resulting in a failed test.

This method was needed to work around problems experienced on Linux or Mac. However, it seems that splitting the navigation tests into their own respective classes (or only setting a context menu on the area when context menu-related tests are running) has removed some kind of interference that made these tests fail on Linux and Mac. Removing this method no longer makes those tests fail.
@JordanMartinez
Copy link
Contributor Author

It seems the referenced issues are currently blocking the last commit I've pushed to this PR...

@JordanMartinez
Copy link
Contributor Author

An update to JUnit fixed the problem. Nice.

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.

2 participants