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

Fixes #1. Have tests inspect an exception's e.value instead of just t… #417

Closed
wants to merge 11 commits into from

Conversation

twongCMU
Copy link

…he exception. pytest 5.0.0 no longer displays the custom text with str(e)

dhuppenkothen and others added 5 commits June 11, 2019 13:58
Fixed release badge

missing line

Fixed identifier

Will the dashes fix things?

Added JOSS back in

Removed the old badges from readme
…he exception. pytest 5.0.0 no longer displays the custom text with str(e)
@twongCMU
Copy link
Author

It looks like the CI tests fail due to #416 and not my changes

@twongCMU
Copy link
Author

Bah. I fixed two issues in the forked branch, but when I submitted the pull request the links now refer to issues in this stingray repo's issue tracker, which is wrong. Anyway, here are the two issues I fixed:
twongCMU#1
twongCMU#2

@matteobachetti
Copy link
Member

Hi @twongCMU, thanks a lot!

I'm not sure that the complete removal of conftest.py is what we want though. The fact that a line there makes everything fail doesn't make the whole file "unneeded".

An example of conftest.py compatible with the new pytest and astropy versions is here:
https://github.com/astropy/package-template/blob/master/packagename/conftest.py

Copy link
Member

@matteobachetti matteobachetti left a comment

Choose a reason for hiding this comment

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

Hi @twongCMU, please see #415 for your second commit. You do not need to delete conftest.py, which is actually useful. I will merge #415 soon. After that, can you rebase to master and re-submit this PR with only the excinfo.value changes?

@pep8speaks
Copy link

Hello @twongCMU! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 124:80: E501 line too long (92 > 79 characters)
Line 126:80: E501 line too long (98 > 79 characters)
Line 133:80: E501 line too long (86 > 79 characters)
Line 136:80: E501 line too long (93 > 79 characters)
Line 156:80: E501 line too long (88 > 79 characters)

Line 15:80: E501 line too long (84 > 79 characters)
Line 16:1: E402 module level import not at top of file

@twongCMU
Copy link
Author

twongCMU commented Jul 2, 2019

Yes, I agree that it is better to keep conftest.py

@twongCMU twongCMU closed this Jul 2, 2019
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

5 participants