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

Some GUI tests #1700

Merged
merged 5 commits into from
Aug 12, 2016
Merged

Some GUI tests #1700

merged 5 commits into from
Aug 12, 2016

Conversation

oscargus
Copy link
Contributor

@oscargus oscargus commented Aug 8, 2016

  • Tests for opening and closing, in several ways, most of the dialogs accessible from the menus.
  • Test for opening and closing the OO/LO style selection dialog
  • Test for enabling/disabling undo/redo

@oscargus oscargus force-pushed the undoredoguitest branch 2 times, most recently from 7297b16 to 9377571 Compare August 8, 2016 23:10
@oscargus oscargus changed the title [WIP] Test enabling/disabling undo/redo Some GUI tests Aug 9, 2016
@oscargus
Copy link
Contributor Author

oscargus commented Aug 9, 2016

Seems like it is hard to get stable GUI tests... I think some of them makes sense and the reasons behind the commented out tests in ParameterizedDialogTests are worth fixing.

How should I progress here? Mark them and @Ignore and hope that eventually things will be working?

@koppor
Copy link
Member

koppor commented Aug 9, 2016

I ❤️ UI tests. We discussed reenabling them on #1524. After reading http://stackoverflow.com/a/4995662/873282, I think it is no harm to make the timer daemonized.

Can you backport my changes at https://github.com/JabRef/jabref/pull/1524/files to your branch? Then, the GUI tests will work on Travis again.

I will also update the Eclipse settings that it won't join lines. I think, the current code formatting is good enough and we should get rid of @formatter:off tags.

What do you mean by "stable" tests? Are the effects always different?

The thing is, we decided to go away from GUI tests, because they always cause trouble and headaches. Our UI code is sometimes a really mess and thus the GUI tests will probably never be stable. We have to switch to JavaFX completely to have a clean and testable GUI code.

@oscargus
Copy link
Contributor Author

"Stable" in the sense that they pass on Travis every time, independent of which other tests are running (and to some extent even locally...).

Although I do not think it makes sense to run all these tests for every PR/build due to the time it takes, they may still be useful to run before a release, including adding more of these simple "just click everything you can and see that nothing bad happens". Both #1710 and #1714 were consequences of these tests and with some slightly more advanced ones, the bugs solved in #1631 and #1649 would never have made it (since they happened simply by clicking on "open file"). So somehow it would, eventually, be good to merge them so that they are available, but at some new location that are not automatically executed at every build. And when it comes to gradle etc I'm lost.

@koppor
Copy link
Member

koppor commented Aug 12, 2016

We have CircleCI for a quick build checking for compilation issues and if JabRef compiles, the binaries are served at builds.jabref.org. Then, it doesn't matter, how long TravisCI runs. All tests should run at each PR so that the author is directly notified about unwanted side effects of his PR. When we run the tests at a release only, it is a much higher effort to track down the cause and we will probably not run the tests at all to safe time during release.

@koppor koppor merged commit 65b3ed8 into JabRef:master Aug 12, 2016
@oscargus
Copy link
Contributor Author

I see the point and I agree that ideally it should be like that, but now the new GUI tests have already started to fail... Still, it makes sense to run these (massive and trivial) tests every now and then just to see that nothing, very unexpected, has happened.

Somehow I see two options: remove them, because they are unstable, or keep them but only run them on occasion. The third option doesn't seem like it can be actually selected at the moment...

@koppor
Copy link
Member

koppor commented Aug 12, 2016

Now, on TravisCI, integration tests still run, but are not treated as breaking the build - 7832ddf

That means, one can run the tests on occasion and if one wants to dive deeper, he can try to solve the issues. Howver, we think, the GUI code might be that broken so that we have to focus on porting JabRef completely to JavaFX. We track the progress at koppor#143. "Broken" means that some GUI code runs outside the EDT.

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

Successfully merging this pull request may close these issues.

2 participants