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

Add skip condition if QT_API is no set (issue #1101) #1105

Merged
merged 5 commits into from May 13, 2020

Conversation

ebjordi
Copy link
Contributor

@ebjordi ebjordi commented Mar 19, 2020

pytest tardis fails on import of gui interface and widgets if QT_API variable is not set

Description

The issue appears on import (and I still don't how skipimport would work but I'll revisit at some point). I changed the call so that the import wouldn't check inside the module until needed for the test by importing the whole module. Following this some of the objects inside the function have to be called more explicitly.

I added the skipif decorator from pytest. The test now skips when the variable QT_API is not defined, but I'm not sure how the pytest tardis should proceed when the variable is defined. When I define QT_API either way on my master branch the test still skips, which I suppose is the expected behaviour because I didn't modify anything there.

On a different commit I modified the functions interface and widgets to change how the Iphyton library calls specific functions, as can be seen this changes a qt4 to a qt5 and is purely for consistency, given that the functions are the same.

Motivation and Context

Adresses #1101

How Has This Been Tested?

Run pytest tardis and the test now skips when QT_API is not declared.
Run again for the Iphyton calls change.
Giving( to save as reference)
263 passed, 1119 skipped, 1 xfailed in 24.38 seconds

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

All input is appreciated.

@ebjordi
Copy link
Contributor Author

ebjordi commented May 8, 2020

@wkerzendorf Could you please reopen this?
I don't have permissions.
Thanks

@wkerzendorf wkerzendorf reopened this May 8, 2020
@ebjordi
Copy link
Contributor Author

ebjordi commented May 8, 2020

Ok, so this is ready to merge I think

Copy link
Contributor

@sashank27 sashank27 left a comment

Choose a reason for hiding this comment

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

You can check in ipython console if the QT_API is set or not. Also, adding -rsx flag in pytest specifies the reason for skipping the test

tardis/gui/tests/test_gui.py Outdated Show resolved Hide resolved
tardis/gui/tests/test_gui.py Outdated Show resolved Hide resolved
ebjordi and others added 2 commits May 10, 2020 02:27
Co-authored-by: Sashank Mishra <sashankmishra27@gmail.com>
Co-authored-by: Sashank Mishra <sashankmishra27@gmail.com>
@marxwillia marxwillia merged commit b6387c9 into tardis-sn:master May 13, 2020
@@ -1066,13 +1066,13 @@ def __init__(self, tablemodel, config=None, atom_data=None, parent=None):
"""

#assumes that qt has already been initialized by starting IPython
#with the flag "--pylab=qt"
#with the flag "--pylab=qt"gut
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix this typo later

atharva-2001 pushed a commit to atharva-2001/tardis that referenced this pull request Oct 1, 2021
Add skip condition if QT_API is no set (issue tardis-sn#1101)
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.

None yet

4 participants