-
Notifications
You must be signed in to change notification settings - Fork 444
Use fixture for QApplication initialization in tests #1065
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
Conversation
So, still not working, but now it's pretty clear the error also observed in #1057 happens when Travis tries initializing Line 10 in a348e30
It again works locally for me, so there's something with the Travis setup that makes the initialization of the QApplication fail. |
That is, the issue is not with pytest, or pytest-cov, as I speculated earlier, but with some of the PyQt dependencies i guess. |
Interesting. Let me pull your branch and see if it fails. Because my pytest-crash branch consistently failed locally |
What platform are you on, by the way, @dybber ? |
I'm on Mac OS X Mojave I'm speculating if it could be some Qt-dependencies that needs to be installed? E.g. should there be an extra |
Downgrading PyQt5 to the previous version seems to help, the most recent version (PyQt 5.15) is less than two weeks old, so perhaps it has a few quirks. |
Now only errors on Mac OS X. I'm not sure what to do there, as it works for me locally. My initial suggestions for what we could try is:
Perhaps that could also be attempted in a separate PR |
Upgrading OS X version, seemed to help - all test cases passed this time, now the problems are some version conflict happening when packaging. The error happens when executing this |
The last problem appeared to be that:
I've added an extra dependency on docutils < 0.16, in the docs section, which solved it. I don't know if that is the best way to solve it, alternatively we could try with briefcase==0.3.0 |
Looks great. My main computer is in use elsewhere at the moment (hosting a Zoom meeting!) but once I've got it back I'll do a review of the changes and -hopefully -- merge. Thank you so much for persevering with this. (I was thinking this morning what a thankless task it is, just to get CI services to succeed when tests pass locally..) |
Great. I just checked that briefcase 0.3.0 no longer depends on boto3, botocore and thus not docutils, so I will just try one more commit to remove the explicit docutils dependency again and instead upgrade briefcase. |
That's unexpected, and again it still builds fine locally on my Mac. |
Just tried with a clean virtualenv locally, and now I get the same issue locally |
Oh, the way to invoke briefcase has changed in v0.3: beeware/briefcase#266 |
I think upgrading to briefcase 0.3 should be a separate issue. It seems that will require a lot of extra work. I just tried, and hit multiple issues. At some point it might be worth doing, but I'm not sure I'm the right one to do it. I will thus go back to version 0.2.9 of briefcase and put in that extra docsutils requirement again |
I don't think I have more to contribute to this one, but please let me know if you have any comments. |
Ok; just took this for a spin locally and no problems on Windows. Thank you so much for working this through. I'm sure it won't be the last version incompat issue we come across, but at least we can now go back to actually making the software do useful things :) |
Great, thanks. Looking forward to seeing some of the "useful things" merged ;-) Btw. on an unrelated not, as a maintainer, could you make a Github label for translations? There's a lot of translation issues, would be nice to get them all tagged with a label. |
re translations, done: translations |
I'm happy to do it via a direct file edit on GH -- saving the aggro of a round-loop PR |
Just tell me what to write there (or do it yourself if you have privs; I'm not sure) |
Thanks, perhaps something like "match xcode version with earliest supported by current PyQt (earliest to maximize compatibility)" |
Done on master; please check if it shows as expected |
Seems fine, thanks. |
Perfect, thanks! |
Perhaps this will fix the Segmentation fault during testing that causing Travis to fail.
EDIT: Forgot to write that all tests that does anything with Qt Widgets, Windows, etc. now needs to specify that they require the "qtapp" fixture in their argument list