Skip to content

Conversation

@wettenhj
Copy link
Contributor

I've noticed that test results are not yet included in the Continuous Integration (CI) builds, e.g. https://github.com/wxWidgets/Phoenix/blob/master/.travis.yml

I tried running the tests myself on Windows 7 with Python 2.7 and found that I needed to tweak a few things to get them to run smoothly - a few test failures is fine while Phoenix is still in development, but we don't want race conditions in the tearDown of one test to impact subsequent tests.

I'm certainly not expecting this branch to be merged in its current form, but I'd be interested in a discussion about including tests in CI builds.

The tests can be seen running in AppVeyor (after my tweaks were applied) here: https://ci.appveyor.com/project/wettenhj/phoenix/build/1.0.7

The test coverage shown here: https://ci.appveyor.com/project/wettenhj/phoenix/build/1.0.7#L5346 could be improved by merging coverage reports from multiple CIs (covering multiple OS's). Codecov.io does this well.

Also the CI build status could be set to fail if some of the tests fail, but I'm not sure if "python build.py test" returns a nonzero exit code if "pytest" does.

I'd be happy to break this pull request up into smaller pull requests if that helps (and squash multiple commits correcting my mistakes into a smaller number of commits).

Also happy to add other Python versions to the AppVeyor CI, but I thought I'd start with just one (2.7.13) to reduce the turnaround time for the Windows CI builds at this early stage.

Feedback is most welcome!

wettenhj added 29 commits March 17, 2017 16:36
You also need to ensure that vcvarsall.bat is in your PATH.
It's typically installed in:
C:\Users\[username]\AppData\Local\Programs\Common\Microsoft\Visual C++ for Python\9.0\
Trying to Show it manually in this test seems to result in:
E       wxAssertionError: C++ assertion "!wxMouseCapture::IsInCaptureStack(this)
" failed at ..\..\src\common\wincmn.cpp(3271) in wxWindowBase::CaptureMouse(): R
ecapturing the mouse in the same window?
…4982bebf5cbb4a883cd42226 and replacing it with a better solution to the problem of

methods invoked by wx.CallAfter being called after the top-level windows have
been destroyed, during tearDown of unittests.
where methods invoked by wx.CallAfter and wx.CallLater try to access destroyed
objects during tearDown of unittests.
by wx.CallAfter to avoid problems with tearDown in unittests.
UltimateListControl test class's tearDown to avoid PyNoAppError.
within quotes.  Also renaming appveyor subdirectory to .appveyor
@wettenhj
Copy link
Contributor Author

BTW, I realized that refusing to run timers unless there is a running MainLoop, as done here: 293783c could break existing wxPython applications, because a lot of stuff can be put in the wx.App's "OnInit" method which runs before the MainLoop begins. So if a timer does need to be avoided in a unit test (to avoid race conditions in WidgetTestCase's tearDown method), you could just do something like this in WidgetTestCase's setUp method: "self.app.thisIsATest = True" and then check for "if hasattr(wx.GetApp(), "thisIsATest") in the widget implementation if you need to avoid starting a timer when the widget was created by a unit test.

@RobinD42
Copy link
Member

Thanks, I'll take a closer look at this when I'm able. Looking at the list of commits a split and/or squash may be helpful if you have the time.

If you haven't already you should also look at #235 which will help with running a test suite in a more isolated wx.App for those cases that have trouble playing nice with others. It's underlying goal is also to get the unittests to the point where they can be used in the CI runs.

Finally, thanks for the VCForPython27 fixes. I've been wanting to dig in to the cause of that problem for a while now. That would be a good candidate for a separate PR.

@wettenhj
Copy link
Contributor Author

Thanks, #235 does look interesting indeed.

I hope to be able to close this PR soon (without expecting it to be merged) after breaking it down into smaller chunks, e.g. see: #257

@wettenhj
Copy link
Contributor Author

wettenhj commented Apr 6, 2017

Now that #255 and #257 have been merged, I'm going to close this. Running tests on CIs can be revisited later in a more refined pull request.

@wettenhj wettenhj closed this Apr 6, 2017
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