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

PR: Install and run window manager in Travis to fix focus issues and unskip now-passing tests #6286

Merged
merged 4 commits into from
Jan 24, 2018

Conversation

CAM-Gerlach
Copy link
Member

I've been noticing consistent problems with tests failing on Travis when focus and other GUI elements are involved. Accordingly, in #6132 @ccordoba12 suggested installing a window manager, as that fixed similar issues with CircleCI in #4151 . Therefore, this is a shot at doing the same for Travis, which would hopefully resolve a ton of frustration I'm having with a the tests in a number of recent PRs. First crack at it will just ensure the code runs without error, and then I'll try enabling tests that failed due to focus/etc. issues to see if this fixes it. However, it can be limited to ensure this PR can get merged quickly; the primary objective is to verify this is actually working and fixes at least some of the problems.

@pep8speaks
Copy link

pep8speaks commented Jan 24, 2018

Hello @CAM-Gerlach! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on January 24, 2018 at 17:28 Hours UTC

@CAM-Gerlach CAM-Gerlach force-pushed the travis-window-manager branch from f55ab7d to bf61226 Compare January 24, 2018 07:26
@CAM-Gerlach CAM-Gerlach changed the title PR: [WiP] Install and run window manager in Travis to avoid focus issues PR: Install and run window manager in Travis to fix focus isues and unskip now-passing tests Jan 24, 2018
@CAM-Gerlach CAM-Gerlach changed the title PR: Install and run window manager in Travis to fix focus isues and unskip now-passing tests PR: Install and run window manager in Travis to fix focus issues and unskip now-passing tests Jan 24, 2018
@CAM-Gerlach
Copy link
Member Author

@ccordoba12 The great news is that after implementing this, testing, and fiddling around a bit, I not only got all three of my previously skipped focus/"realistic" UI tests passing consistently on all builds (but one), but also got test_values_dbg and the infamous test_calltip working just fine without any timeout problems on both Travis and Python 2, with zero failures (either flaky or full suite reruns) across the full suite of builds (again, with one exception) during 2-3 full run-throughs. In total, this increases coverage by almost a full 1%. Also, I did a bit of associated minor cleanup and timeout refinements.

However, the one remaining issue is that the Python 3.5 PyQt5 build consistently segfaults every time as soon as the test collection phase begins before any tests are collected, which started as soon as I added the few lines to load the window manager (before unskipping any tests or making any other changes); I can only assume it is no accident that this build is the one with different setup procedures, using pip instead of conda to install packages, though I'm not sure exactly why this would make it segfault immediately at the start of test collection, before even really doing anything. Any insight? It would be a rather kludgy and awkward to maintain "fix" to disable the window manager and the now-failing-again tests on just this build, so hopefully there's a better solution. Thanks!

@@ -120,7 +121,7 @@ def test_find_number_matches(qtbot):

def test_move_current_line_up(editor_bot):
editor_stack, editor, qtbot = editor_bot

Copy link
Member

@ccordoba12 ccordoba12 Jan 24, 2018

Choose a reason for hiding this comment

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

Please revert all blank space removals you introduced in this file.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, accidentally left "remove trailing whitespace when saving" on...would be nice to have a toggle somewhere in the UI to make it less of a pain to be going in an out of preferences to keep changing/verifying it. Fortunately, it happened to be the last thing in my undo history so it has easy to revert.

reason="It times out in our CIs")
@pytest.mark.skipif(os.environ.get('CI', None) is not None and os.name == 'nt',
reason="It times out on AppVeyor.")
@pytest.mark.timeout(timeout=20, method='thread')
Copy link
Member

@ccordoba12 ccordoba12 Jan 24, 2018

Choose a reason for hiding this comment

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

Please add @pytest.mark.slow

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup.

@ccordoba12
Copy link
Member

In total, this increases coverage by almost a full 1%. Also, I did a bit of associated minor cleanup and timeout refinements.

Wow, this is great @CAM-Gerlach! Thanks a lot!

However, the one remaining issue is that the Python 3.5 PyQt5 build consistently segfaults every time as soon as the test collection phase begins before any tests are collected

Yep, I fixed that already in PR #6289. So please address my comment, do a rebase to pick up #6289 and then this one will be ready ;-)

@CAM-Gerlach CAM-Gerlach force-pushed the travis-window-manager branch from bf61226 to ecc3488 Compare January 24, 2018 17:28
@CAM-Gerlach
Copy link
Member Author

@ccordoba12 Thanks! Should be good to go now.

@spyder-ide spyder-ide deleted a comment from CAM-Gerlach Jan 24, 2018
@ccordoba12 ccordoba12 merged commit a7394fb into spyder-ide:3.x Jan 24, 2018
ccordoba12 added a commit that referenced this pull request Jan 24, 2018
@CAM-Gerlach CAM-Gerlach deleted the travis-window-manager branch January 30, 2018 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants