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

Proposal: Adopt the pytest library for testing #212

Closed
4 tasks
csadorf opened this issue Jul 24, 2019 · 7 comments · Fixed by #275
Closed
4 tasks

Proposal: Adopt the pytest library for testing #212

csadorf opened this issue Jul 24, 2019 · 7 comments · Fixed by #275
Assignees
Labels
enhancement New feature or request

Comments

@csadorf
Copy link
Contributor

csadorf commented Jul 24, 2019

Proposal

I propose to adopt the pytest library as the main testing framework as a replacement for the Python built-in unittest package.

Pros

  • Writing tests is easier.
  • The pytest command line tool is easier to use and more powerful.
  • The pytest output is much more descriptive.

Cons

  • We add a dependency to the testing workflow.

Migration

Adopting pytest as the main testing framework would be very simple. In fact, it is already possible to execute all tests with pytest without the need to modify the signac source code or test code at all.
The following command executed within the signac package root directory will execute all tests:

~/signac $ pytest

These are the steps we would need to take:

  1. Updates the dev-requirements to include pytest.
  2. Update the circle-ci test commands.
  3. Update the documentation on how to run tests.
  4. (optional) Use unittest2pytest to rewrite existing tests.
@csadorf csadorf added the enhancement New feature or request label Jul 24, 2019
@csadorf
Copy link
Contributor Author

csadorf commented Jul 24, 2019

@glotzerlab/signac-maintainers @mikemhenry I'd like your opinion on this.

@bdice
Copy link
Member

bdice commented Jul 24, 2019

Pytest is nice, but I'd consider the development time / maintenance costs of rewriting tests / potential benefits before diving in.

@csadorf
Copy link
Contributor Author

csadorf commented Jul 25, 2019

That's why I'm putting this up for debate. I don't think there are any major transition costs, since we don't have to rewrite any tests at all. I'd argue that maintenance will go down, since writing new tests will be simpler.

If at all, we just use automated tools to rewrite tests, but we probably need to make a few manual adjustments.

@vyasr
Copy link
Contributor

vyasr commented Jul 25, 2019

Bradley and I have discussed this possibility before for freud. Pytest is nice, but I haven't felt that the benefits merited the extra dependency. It does make writing new tests a bit shorter, but at this point writing tests with unittest just involves copying the boilerplate from another test, so it's basically zero effort. I don't think that rewriting our existing tests to use the pytest framework is a worthwhile investment of time, but I assume you would propose to just leave the existing tests in place (since pytest can process the vast majority of unittest features) if the autoconvert packages don't work well?

In general, I've largely heard good things about pytest. The main issue I've heard is that their development cycle is quick, so deprecations and removals of features were not infrequent over the past few years. I don't know if that's still the case though; their changelog looks reasonably stable at this point. I would prefer not to deal with deprecations in our testing framework, but that may no longer be an issue.

Tl;dr: I'm mostly neutral on this question, aside from the fact that I'd want to be very sure that the benefits are worth the time investment before trying to switch over.

Whoops I typed this all out yesterday and just realized I never posted, sorry about that.

@mikemhenry
Copy link
Collaborator

I need to re-adjust my github notification settings...

I really like pytest but don't have much experience with unittest so I don't really have much comparison information to add and I'm sure if I talk about how useful the test parameterization is and how its easy to monkey patch, the response will be "but unittest does that too.

I don't care that its not in the standard library, but I don't want to change it just because I like pytest more. Since we could change it now, maybe as soon as there is a compelling reason/feature in pytest, we switch?

@vyasr
Copy link
Contributor

vyasr commented Jan 9, 2020

@mikemhenry @csadorf @bdice I'm revisiting this topic in view of #249. I started a bit about that PR and prototyping code to make sure the classes I'm suggesting make sense, and I think that pytest will make it quite a bit easier to write those tests. I've been using it extensively for a different package, and I have definitely found that it simplifies test writing enough that I'm in favor of making this switch. I made a PR a while ago that fixed the only tests that failed using pytest, so I am in favor of pushing forward with this change if you all approve. I think rewriting existing unit tests is a lower priority, though, and can be done at a later point (e.g. using unittest2pytest).

@vishav1771
Copy link
Contributor

Assign me this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants