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: Mark introspection test slow and re-enable it on certain CIs #6138

Merged
merged 4 commits into from
Jan 6, 2018

Conversation

CAM-Gerlach
Copy link
Member

A one line PR: Just mark the editor_introspection test as slow, as due to the timeouts required to make it pass reliably on all systems, it is the slowest in the entire suite, taking over 30s just by itself. Therefore, mark it as such.

@CAM-Gerlach CAM-Gerlach added this to the v3.2.6 milestone Jan 5, 2018
@CAM-Gerlach CAM-Gerlach requested a review from ccordoba12 January 5, 2018 11:29
@CAM-Gerlach
Copy link
Member Author

Is there a way to disable codecov for a specific PR? Couldn't find it...both this one and #6137 its failing, even though clearly neither add any real new executable code to the project.

@@ -58,6 +58,7 @@ def get_spyder_pythonpath(*args):
editor.introspector.plugin_manager.close()


@pytest.mark.slow
@pytest.mark.skipif(os.environ.get('CI', None) is not None,
reason="This test fails too much in the CI :(")
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this condition to see if it passes in our CIs because else this test wouldn't be run anywhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes, silly me. I tested it previously with #6044 since it was failing locally too, but after adding huge qtbot.wait()s I got it to pass, though still not perfectly consistently. I figured that had a good chance at fixing it on the CIs, but at the cost of a minimum of 30+ seconds added to every build in the best case; if it failed (which just happened sometimes maybe 1 out of 10 on my machine) and reran 3x, that would multiply to 90, and with 3 flaky runs, (which never seemed to change the outcome and just triple the runtime when it failed, so I dropped them in #6099 ) it would be around 5 extra minutes of build time worst case, before failing anyway. That said, doesn't hurt giving it a shot...

@ccordoba12 ccordoba12 modified the milestones: v3.2.6, v3.2.7 Jan 5, 2018
@ccordoba12
Copy link
Member

Please rebase this PR in top of latest 3.x and skip test_introspection in Python 2 to avoid some strange segfaults with other tests.

@jnsebgosselin
Copy link
Member

Is there a way to disable codecov for a specific PR? Couldn't find it...both this one and #6137 its failing, even though clearly neither add any real new executable code to the project.

I've raised the treshold of codecov from 0% to 1% before failing. But it is in #6095, which has not been merged yet. Sorry for the inconvenience.

@CAM-Gerlach CAM-Gerlach force-pushed the mark-introspection-slow branch from b471e0e to 1ce2ee6 Compare January 6, 2018 03:30
@CAM-Gerlach
Copy link
Member Author

Please rebase this PR in top of latest 3.x and skip test_introspection in Python 2 to avoid some strange segfaults with other tests.

Done and done. However, this time the Py3.5/PyQt4 test failed in the same manner; a segfault a few tests after the introspection one (though not on the same test I believe). Maybe some kind of persistent object is sticking around causing trouble? I restarted the previous tests that failed just to see if it was reproducible, and sure enough they failed again.

No idea what's really going on, but it seems like it fails on both Py2 and Py3, and PyQt4 and PyQt5, so not sure any build is safe. It seems this one test is just nothing but trouble...perhaps it is almost better to both mark it as slow, and skip it at least on Linux + CI (i.e. Travis), just let AppVeyor run it along with users if they want to (with the right flag to run slow tests, presumably).

I've raised the treshold of codecov from 0% to 1% before failing. But it is in #6095, which has not been merged yet. Sorry for the inconvenience.

No worries. I just didn't want @ccordoba12 to think the actual tests were failing or something.

@ccordoba12
Copy link
Member

ccordoba12 commented Jan 6, 2018

No idea what's really going on, but it seems like it fails on both Py2 and Py3, and PyQt4 and PyQt5

Let's try it one more time, but now please skip it in PyQt4. I think Travis should pass this time.

@ccordoba12 ccordoba12 modified the milestones: v3.2.7, v3.2.6 Jan 6, 2018
@ccordoba12
Copy link
Member

Now things pass in Travis and AppVeyor, and with a nice increase in coverage! Sweet!

@pytest.mark.skipif(os.environ.get('CI', None) is not None,
reason="This test fails too much in the CI :(")
@pytest.mark.slow
@pytest.mark.skipif(PY2 or PYQT4,
Copy link
Member

Choose a reason for hiding this comment

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

Last time: please remove PY2 from here.

Copy link
Member

Choose a reason for hiding this comment

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

I think the failures only happen in PyQt4.

Copy link
Member Author

Choose a reason for hiding this comment

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

But it segfaulted on Py2 + PyQt5 too... ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I can't see it now because I rebased it, but I'm 99% sure both Python tests failed, the QtPy4 one with segfault and the QtPy5 just hung and stopped, and then I restarted the QtPy5 one and it segfaulted too. But if you're sure, I can resend it again...

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, here you go. I cancelled midway but it did indeed segfault.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, not prob. Thanks for trying 👍

@ccordoba12 ccordoba12 merged commit ca27d3d into spyder-ide:3.x Jan 6, 2018
ccordoba12 added a commit that referenced this pull request Jan 6, 2018
@CAM-Gerlach CAM-Gerlach changed the title PR: Mark introspection test slow as its the slowest in the entire suite PR: Mark introspection test slow and re-enable it on certain CIs Jan 6, 2018
@CAM-Gerlach CAM-Gerlach deleted the mark-introspection-slow branch January 10, 2018 06:42
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.

3 participants