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

Make tests.py into proper unit test, and add Qt version info to pytest header #28

Merged
merged 8 commits into from Jun 9, 2016

Conversation

astrofrog
Copy link
Contributor

Since I'm adding actual tests in #25 and #27, I thought it would make sense to formally switch over to using py.test to run tests. This also makes it so that the py.test header includes detailed Qt version information:

============================= test session starts ==============================
platform darwin -- Python 3.5.1, pytest-2.8.5, py-1.4.31, pluggy-0.3.1
benchmark: 3.0.0 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=5.00us max_time=1.00s calibration_precision=10 warmup=False warmup_iterations=100000)

PyQt4: PyQt: 4.11.4 - Qt: 4.8.7
PyQt5: not installed
PySide: not installed
...

@astrofrog astrofrog changed the title WIP: Make tests.py into proper unit test, and add Qt version info to pytest header Make tests.py into proper unit test, and add Qt version info to pytest header Jun 9, 2016
@astrofrog
Copy link
Contributor Author

This is now ready for review! (I think the quantifiedcode warnings can be ignored in this case)

@goanpeca
Copy link
Member

goanpeca commented Jun 9, 2016

Great work @astrofrog, :-)

@ccordoba12
Copy link
Member

@astrofrog, thanks a lot for working on this! It's a great improvement :-)

@goanpeca, please disable QuantifiedCode.

@ccordoba12 ccordoba12 added this to the v1.1 milestone Jun 9, 2016
assert_pyqt4()
elif QT_API == 'pyqt5':
assert_pyqt5()
else:
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to enter this part if we set always set USE_QT_API in travis.yml?

Copy link
Contributor Author

@astrofrog astrofrog Jun 9, 2016

Choose a reason for hiding this comment

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

Not on Travis or AppVeyor, but it's possible if one runs the tests locally without specifying USE_QT_API or QT_API

Copy link
Member

Choose a reason for hiding this comment

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

Ok

@astrofrog
Copy link
Contributor Author

@ccordoba12 - do you need me to squash the commits? (not sure what kind of policy you use for qtpy compared to other projects)

@goanpeca
Copy link
Member

goanpeca commented Jun 9, 2016

@ccordoba12 the quantified code suggestions are valid

  • Consider documenting your function(s)
  • Avoid None as a redundant second argument to dict.get()

I will disable the one that complaints about having non pythonic method names which does not work when using Qt.

@astrofrog could you make the suggested "fixes"?

@ccordoba12
Copy link
Member

Ok, I think this one is ready. We have to merge it with AppVeyor failing because there's no AppVeyor working at this point :-)

@ccordoba12 ccordoba12 merged commit c5dbd0b into spyder-ide:master Jun 9, 2016
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

3 participants