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.

Sign up for GitHub

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

Jump to bottom

Switch to pytest #850

Merged
merged 6 commits into from Apr 18, 2020
Merged

Switch to pytest #850

merged 6 commits into from Apr 18, 2020

Conversation

stephenfin
Copy link
Contributor

@stephenfin stephenfin commented Mar 31, 2020 •
edited

From the nose website:

Nose has been in maintenance mode for the past several years and will likely cease without a new person/team to take over maintainership. New projects should consider using Nose2, py.test, or just plain unittest/unittest2.

pytest is a good option that gives us much nicer output and makes running individual tests significantly easier. It's also faster. Much faster. On my local machine, current test execution drops by from ~430sec to ~330sec, and if the xdist plugin is enabled this drops even further to ~130sec.

@stephenfin stephenfin mentioned this pull request Mar 31, 2020
Closed
@stephenfin stephenfin force-pushed the pytest-migration branch 2 times, most recently from 08c021f to 8f65d0a Compare March 31, 2020 22:08
@akrabat
Copy link
Member

akrabat commented Apr 1, 2020

I suspect that this relates to #838.

@stephenfin
Copy link
Contributor Author

stephenfin commented Apr 1, 2020 •
edited

I suspect that this relates to #838.

Damn, I didn't realise there was prior art here. However, it looks like #838 is stalled and this is good to go (it's passing CI), so perhaps we should continue with this? Do my changes make sense?

@stephenfin
Copy link
Contributor Author

stephenfin commented Apr 1, 2020 •
edited

I do like the approach @normanlorrain used though. I think that's based on this doc. I could adopt tests to match that in a later PR, if it makes sense?

@stephenfin
Copy link
Contributor Author

I just went and used the approach suggested in #838 since it was cleaner than mine. Hopefully this makes sense now.

@akrabat
Copy link
Member

akrabat commented Apr 2, 2020 •
edited

I'm speaking at a virtual conference this weekend, so may not get to it until next weekend.

@lornajane
Copy link
Contributor

This is great, I think people really expect to be able to type pytest and have the expected thing happen, so I'm very happy to see this. Should we include pytest in the requirements file or do you think people will mostly have it installed anyway?

For me, the fancytitles test fails (situation normal) but all my sphinx tests also fail:

Running Sphinx v1.8.5

Extension error:
Could not import extension rst2pdf.pdfbuilder (exception: No module named 'rst2pdf')

I'm not sure if this is directly linked to this PR though since my python has seemed confused about where rst2pdf is when I try to use the local source version of it.

(@akrabat can you poke travis to repeat the build? It succeeded but I don't have rights to make it go again)

@stephenfin
Copy link
Contributor Author

For me, the fancytitles test fails (situation normal) but all my sphinx tests also fail:

Running Sphinx v1.8.5

Extension error:
Could not import extension rst2pdf.pdfbuilder (exception: No module named 'rst2pdf')

I'm not sure if this is directly linked to this PR though since my python has seemed confused about where rst2pdf is when I try to use the local source version of it.

Ah, these now expect the rst2pdf package to be installed itself. Could you try creating a new virtualenv and use the installation instructions from the .travis.yml file? That should get things going.

@stephenfin
Copy link
Contributor Author

As for including pytest in the requirements, sure. I only omitted it because nose wasn't included previously. I'll tackle that as a follow up PR though

@stephenfin
Add pytest-based test infrastructure
b5a7549 
This is inspired by pytest's "Working with non-python tests" guide [1].
In short, we iterate through the 'input' directory and attempt to find
either files that end in '.txt' (meaning a test that should be run
through the 'rst2pdf' binary) or directories that start with 'sphinx-'
and contain a 'conf.py' file (meaning a test that should be run through
the 'sphinx-build' binary). In both cases, we build the PDFs and compare
checksums, just as we were doing before.

We *haven't* migrated a lot of the tooling to add additional checksums,
but that's okay since we can still do this manually and we plan to
switch to image-based comparisons in the near future.

[1] https://docs.pytest.org/en/5.4.1/example/nonpython.html

Signed-off-by: Stephen Finucane <stephen@that.guru>
Suggested-by: Norman Lorrain <normanlorrain@gmail.com>
@stephenfin
docs: Update documentation to reference pytest
f114768 
Expose the new shiny.

Signed-off-by: Stephen Finucane <stephen@that.guru>
@stephenfin
travis: Use pytest-based tests
1b109af 
Signed-off-by: Stephen Finucane <stephen@that.guru>
@stephenfin
Remove old nose-based test infrastructure
10e81f7 
This removes a lot of the functionality that allowed us to update
checksums, but we want to replace that testing style with visual diffs
so this isn't a big deal.

Signed-off-by: Stephen Finucane <stephen@that.guru>
@stephenfin
Copy link
Contributor Author

stephenfin commented Apr 18, 2020 •
edited

(@akrabat can you poke travis to repeat the build? It succeeded but I don't have rights to make it go again)

I've rebased and force pushed this branch in the hopes of stirring Travis into action. Hopefully that does the trick.

@akrabat Anything else you need from me here?

EDIT Yup, Travis is happy once again 🥳

@akrabat akrabat self-requested a review April 18, 2020 13:02
@akrabat
Copy link
Member

akrabat commented Apr 18, 2020

I had a failure on sphinx-multidoc. Manual validation shows that files produced are correct, so I've added the hash my Mac created.

Good to merge once Travis passes.

@akrabat akrabat merged commit 4840fde into rst2pdf:master Apr 18, 2020
@akrabat akrabat mentioned this pull request Apr 18, 2020
Closed
@akrabat akrabat added this to the 1.0 milestone Apr 18, 2020
@stephenfin stephenfin deleted the pytest-migration branch April 19, 2020 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akrabat akrabat akrabat approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.97
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@stephenfin @akrabat @lornajane