search

mdshw5 / pyfaidx

Efficient pythonic random access to fasta subsequences
https://pypi.python.org/pypi/pyfaidx
Other
459 stars 75 forks source link

Minor change in pytest xfail usage in tests #191

Closed ap-- closed 2 years ago

ap-- commented 2 years ago

Hi @mdshw5,

This PR is very much optional. I noticed that you were using pytest.mark.xfail to mark tests that raise specific Exceptions. This might be my OCD talking, but it's not the intended usage for xfail, which should be used to mark tests that either fail due to missing implementations or because they are expected to fail due to other reasons: https://docs.pytest.org/en/6.2.x/skipping.html

If the code should raise an Exception due to wrong input or other mistakes it's better to use pytest.raises.

In your test-suite it seemed to me that only two tests qualified for being marked as xfail. Again, this is a very minor distinction. Feel free to close the PR if you prefer using xfail.

Cheers, Andreas :smiley:

mdshw5 commented 2 years ago

Thanks for sorting this out. I was confused about expected failures vs expected exceptions when I made the transition to pytest. I'll gladly merge this PR.