Skip to content

Python 1297 add support for pytest for unit tests #1171

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

Closed
wants to merge 5 commits into from

Conversation

bschoening
Copy link
Contributor

@bschoening bschoening commented Jul 8, 2023

Updated test_host_connection_pool.py to use the magic attribute test = False (abstract class) and test = True on derived classes. Nose uses a leading underscore in the class name for similar functionality.

NOTE: the appveyor tests fail on the use of Python 3.6 f-string syntax. PR 1168 resolves this by changing appveyor.yaml to switch from Python 3.5 to 3.7.

@absurdfarce absurdfarce self-requested a review July 20, 2023 05:22
@bschoening
Copy link
Contributor Author

@absurdfarce any update on this? It's only a couple of lines of code.

@absurdfarce
Copy link
Collaborator

absurdfarce commented Nov 13, 2023

Apologies @bschoening, I've been driving hard to try and get 3.29.0 out the door and this one slipped past me.

My original hope was to get PYTHON-1297 (which replaces nose with pytest) into 3.29.0 but we had to abandon that due to a pressing need to get 3.29.0 out. We have done quite a bit of work internally on PYTHON-1297 but that work isn't complete yet; at the moment it's the integration tests that are giving us the most problem. And I'm reluctant to take steps (such as documentation changes) which suggest a change from nose to pytest until we're able to reliably run the entire test suite without issue.

We're trying to figure out if there's an easy way we can collaborate with you on this work without opening our Jenkins instance up to the world. I don't know if we'll be able to do so but I'll post updates here if anything comes of that.

@bschoening
Copy link
Contributor Author

@absurdfarce Ok, I wasn't aware there was a problem with converting Nose integration tests.

Pytest is largely compatible with Nose test, but contributors such as myself can't run pytest because of the classnames in test_host_connection_pool.py. I've also never gotten nose installed and working.

So, a middle ground might be to commit the changes in test_host_connection_pool.py. That would let anyone with a PR run the pytests and still work with nose.

@bschoening
Copy link
Contributor Author

@absurdfarce updated for compatibility with both nodestests and pytest.

@absurdfarce
Copy link
Collaborator

Closing this in favor of #1215 which will represent our complete notion of pytest compat going forward. @bschoening that branch should have all of this work in it already so if you see something missing please let me know. Obviously I very much welcome any additional feedback you may have as well.

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.

2 participants