Skip to content

Conversation

@jeffwidman
Copy link
Contributor

@jeffwidman jeffwidman commented Oct 29, 2018

This migrates tests from unittest to pytest.

I only migrated tests that will continue to be used even after the
removal of the old Simple* classes in #1196.

The motivation for this work was threefold:

  1. Reduce the diff size for Remove old SimpleClient / SimpleProducer / SimpleConsumer #1196
  2. Continue to familiarize myself with the testing code
  3. Improve readability -- pytest is generally more readable unittest

This change is Reviewable

@jeffwidman jeffwidman force-pushed the migrate-from-unittest-to-pytest branch 3 times, most recently from d1acb54 to f99eea9 Compare October 29, 2018 07:26
@jeffwidman jeffwidman force-pushed the migrate-from-unittest-to-pytest branch 2 times, most recently from d6299ed to 8fbbbb3 Compare October 29, 2018 08:38
I only migrated tests that will continue to be used even after the
removal of the old `Simple*` classes in #1196.

The `decorator` in tox is leftover from a previous usage that was
already removed from the code but not from tox.
These fixtures and helper functions were either unused or easily replaced.
@jeffwidman jeffwidman force-pushed the migrate-from-unittest-to-pytest branch 2 times, most recently from f918ec7 to 5e58437 Compare October 29, 2018 09:30
@jeffwidman jeffwidman force-pushed the migrate-from-unittest-to-pytest branch from 5e58437 to 9eb63e1 Compare October 29, 2018 09:34
@tvoinarovskyi tvoinarovskyi self-requested a review October 29, 2018 21:05
Copy link
Collaborator

@tvoinarovskyi tvoinarovskyi left a comment

Choose a reason for hiding this comment

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

LGTM

@dpkp
Copy link
Owner

dpkp commented Nov 10, 2018

radness!

@dpkp dpkp merged commit bb5bc1f into master Nov 10, 2018
@jeffwidman jeffwidman deleted the migrate-from-unittest-to-pytest branch November 12, 2018 16:55
@jeffwidman
Copy link
Contributor Author

Thanks. I hadn't merged because I was planning to migrate a couple more fixtures just finished just yet, but I guess I didn't mention that anywhere. 😄

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.

4 participants