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
Convert unit tests to pytest #96
Conversation
This makes it possible to test with pytest.
This is a fairly basic conversion; this test set is ideally suited for use with pytest fixtures.
Prevents pytest from trying to run the example code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff!
assert_false(items[1].is_immediate) | ||
assert_equal(payload, bytearray(items[1])) | ||
assert items[0].id == 0x1600 | ||
assert items[0].is_immediate is True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is True
? Can it be non-boolean? I guess the strictness gives a bit of extra protection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, just ensures that it's really boolean rather than a truthy non-boolean.
async def test_async_iter(self): | ||
tp = spead2.ThreadPool() | ||
queue = spead2.InprocQueue() | ||
sender = spead2.send.InprocStream(tp, queue) | ||
sender = spead2.send.InprocStream(tp, [queue]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did this interface change too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old code is a deprecated interface, and pytest helpfully reports DeprecationWarnings which is how I spotted it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll keep that in mind for katpoint too, thanks for the tips.
], | ||
python_requires='>=3.6', | ||
test_suite='nose.collector', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated question: so pytest
doesn't need a test_suite
entry?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's more that 'setup.py test' is deprecated (e.g. see pytest-dev/pytest#5534).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right. I've just stumbled on that in NumPy's setup.py
as I was trying to reach enlightenment 🙂
Merged manually (by merging a downstream branch), so closing. |
This is a relatively lightweight conversion, mainly to move from asynctest (which doesn't seem to be maintained) to pytest-asyncio. Unittest assertions are replaced by assertion statements, but the class structure is still in place even though a lot of it would be more gracefully handled by parametrisation and fixtures.
The directory structure is also not changed, and that causes some problems with pytest testing the local source tree rather than the installed version. I'll fix that in a follow-up PR so that I'm not mixing code changes with renames.