switch to pytest for processor/middleware/external tests (not webapp)

RESOLVED FIXED

Status

RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: willkg, Assigned: willkg)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(14 attachments)

44 bytes, text/x-github-pull-request
Details | Review | Splinter Review
53 bytes, text/x-github-pull-request
Details | Review | Splinter Review
53 bytes, text/x-github-pull-request
Details | Review | Splinter Review
53 bytes, text/x-github-pull-request
Details | Review | Splinter Review
53 bytes, text/x-github-pull-request
Details | Review | Splinter Review
53 bytes, text/x-github-pull-request
Details | Review | Splinter Review
53 bytes, text/x-github-pull-request
Details | Review | Splinter Review
53 bytes, text/x-github-pull-request
Details | Review | Splinter Review
53 bytes, text/x-github-pull-request
Details | Review | Splinter Review
53 bytes, text/x-github-pull-request
Details | Review | Splinter Review
53 bytes, text/x-github-pull-request
Details | Review | Splinter Review
53 bytes, text/x-github-pull-request
Details | Review | Splinter Review
53 bytes, text/x-github-pull-request
Details | Review | Splinter Review
53 bytes, text/x-github-pull-request
Details | Review | Splinter Review
Currently, Socorro uses nose for tests. If we switch to pytest, we get the following:

1. a well-maintained test discovery and run tool with a large ecosystem of plugins

2. support for fixtures and some other things that let us build a library of testing tools making it much easier to write, verify, and maintain our tests going forward

3. less test code--we've got over 28k lines of code in our unittests and I claim we could switch to pytest and cut that substantially


This bug covers switching.
I've converted other projects. pytest includes some things that make it easier to convert from nose to pytest and we can write some shims to cover whatever is left.

We've got multiple "test suites". I'm thinking we do the webapp first then the Socorro unittests.

For each of these test suites, we'd switch from nose to pytest, then iterate on fixes until the tests pass shimming as much as we can. For example, we can add eq_ and ok_ shims so we don't have to change everything.

Regarding impact, I think this is a big cleanup effort, but I think it's worth doing. We're pulling components out of Socorro and turning them into separate repositories, but we're going to leave a large chunk of stuff. Making our test situation better makes it easier to extract components out of Socorro and reduces maintenance work for what's left.

I think if this is a project that could get done in a week in a way that's easy to review, it's probably worth it.
+1

Switching the webapp first will require that we switch from django-nose to django-pytest too. Not sure what that means exactly. Regular tests in socorro/unittest/* are just plain classes and functions that start with "Test" (plus it uses nose.tools.ok_ etc but that'd be easy to shim around). 

Another benefit with pytest over nose is that you can have a flake8 plugin [0] so that we can do flake8 checks as part of the test running instead of that having to be a separate CI process. 

[0] https://github.com/mozilla/tecken/blob/master/pytest.ini#L3
Peter pointed out that the web app has a separate test suite than the rest of Socorro and we should split this bug in two.

Given that, I'm rescoping this bug down to switching to pytest for everything except the webapp. We'll do the webapp in a different bug.
Summary: switch to pytest for all tests → switch to pytest for processor/middleware/external tests (not webapp)
PR mozilla/socorro#3787 does the initial test runner switch. After that lands, we have the following things to do:

1. convert eq_ and ok_ and assert_raises to pytest equivalents

   https://docs.pytest.org/en/latest/assert.html
   https://docs.pytest.org/en/latest/assert.html#assertions-about-expected-exceptions

2. remove the home-grown skip_if and replace with pytest equivalent

   https://docs.pytest.org/en/latest/skipping.html

3. (optional) switch the crazy amount of mocking we have to pytest fixtures where convenient


My first priority is to keep the number of differences between the test code in Socorro and the new processor as minimal as possible to reduce the amount of work I have to do to keep up with Socorro changes.

My secondary priority is to clean up the Socorro tests to reduce maintenance burden.

Towards that, I think the first items will reduce the amount of code converting we have to do between Socorro and the new processor, so I want to do those soon especially for processor-related things.

The third we can stave off for now, but I might implement some things in the new processor and backport them to Socorro to keep the number of differences small.
Assignee: nobody → willkg
Status: NEW → ASSIGNED

Comment 6

a year ago
Commit pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/9ff1852a2b3efb6fcce8a7530c7d0069f40390ce
Bug 1361764 - initial switch to pytest (#3787)

* Bug 1361764 - initial switch to pytest

This switches the test runner from nose to pytest for socorro/unittest/
tests (i.e. not the webapp tests).

This doesn't get rid of nose completely--we still have to go through and remove
ok_ and eq_ instances. You can continue to use nose if you like.

This fixes tests that were broken in pytest so that they work with both pytest
and nose.

* Fix env vars for local running

* Fixed tests when ran locally, without any env variables.

* Now using pytest-env instead.
Created attachment 8875851 [details] [review]
pr 3808 - remove skip_if

Comment 8

a year ago
Commit pushed to master at https://github.com/mozilla-services/socorro

https://github.com/mozilla-services/socorro/commit/356d78b15df8300fdeb4361ac50b5cbd748614eb
Bug 1361764 - remove skip_if stuff (#3808)

* Bug 1361764 - remove skip_if stuff

Only one test is skipped. This removes all the skip_if infrastructure and
switches that test to use the pytest skipif decorator.

* Fix "make dockertest" file copy

This fixes the "rm -rf /app/*" line which wasn't removing all the files in
/app/ because the line needs to run in a shell for globbing to work.
Created attachment 8894522 [details] [review]
pr 3891 - update signature generation tests

Comment 10

a year ago
Commits pushed to master at https://github.com/mozilla-services/socorro

https://github.com/mozilla-services/socorro/commit/c084603a62302a42aa422704e200927bd2248ba7
bug 1361764 - convert signature tests to pytest format

This nixes ok_, eq_, and friends for pytest equivalents.

https://github.com/mozilla-services/socorro/commit/9fdb2a3f2406b352c0cf1cb16edcf37eb765ffea
Merge pull request #3891 from willkg/1361764-pytest-sigtests

bug 1361764 - convert signature tests to pytest format

Comment 11

a year ago
Commits pushed to master at https://github.com/mozilla-services/socorro

https://github.com/mozilla-services/socorro/commit/24adcac4adfb5985eda7768a0225f2e195483603
Revert "bug 1361764 - convert signature tests to pytest format"

https://github.com/mozilla-services/socorro/commit/7681a91c1211e294c0d37de2011c4ba8a11a3e0c
Merge pull request #3892 from mozilla-services/revert-3891-1361764-pytest-sigtests

Revert "bug 1361764 - convert signature tests to pytest format"
Created attachment 8895093 [details] [review]
pr 3901 - update app tests

Comment 13

a year ago
Commit pushed to master at https://github.com/mozilla-services/socorro

https://github.com/mozilla-services/socorro/commit/6a3326b53637e9421e9bc4c8ad6d21b2dd73dcd6
bug 1361764 pytest sigtests 2 (#3896)

* bug 1361764 - convert signature tests to pytest format

This nixes ok_, eq_, and friends for pytest equivalents.

* Reworked multi-line asserts to be more readable
Created attachment 8901915 [details] [review]
pr 3945 - update submitter tests
Created attachment 8902460 [details] [review]
pr 3951 - update lib tests

Comment 18

a year ago
Commit pushed to master at https://github.com/mozilla-services/socorro

https://github.com/mozilla-services/socorro/commit/5a30b4d2ef2702982983beff7d9111b3a38a2e72
bug 1361764 - update lib/ to pytest [no rush] (#3951)

* bug 1361764 - update lib/test_converters.py to pytest

* bug 1361764 - update lib/test_datetimeutil.py to pytest

* bug 1361764 - update lib/test_external_common.py to pytest

* bug 1361764 - update lib/test_task_manager.py to pytest

* bug 1361764 - update lib/test_search_common.py to pytest

* bug 1361764 - update lib/test_threaded_task_manager.py to pytest

* bug 1361764 - update lib/test_vertools.py to pytest

* bug 1361764 - update lib/test_transform_rules.py to pytest

* Remove some redundant assertions

* Adjust normalize test removing ver2 which isn't used
Created attachment 8905171 [details] [review]
pr 3962 - update siglists/test_siglists.py and database/test_transaction_executor.py

Comment 21

11 months ago
Commit pushed to master at https://github.com/mozilla-services/socorro

https://github.com/mozilla-services/socorro/commit/6740873f2656847fc78f30a14106eb90f81d233a
bug 1361764 - update unittest/cron/jobs/* to pytest [no rush] (#3978)

* bug 1361764 - update unittest/cron/jobs/* to pytest

* Remove the unneeded sorted() call
Created attachment 8908734 [details] [review]
pr 3978 - update cron/jobs/test* to pytest
Created attachment 8908750 [details] [review]
pr 3995 - update external/boto/test_* to pytest
Created attachment 8908752 [details] [review]
pr 3996 - update external/http/test_* to pytest
Created attachment 8908788 [details] [review]
pr 3997 - update external/test_crashstorage_base.py to pytest
Created attachment 8910364 [details] [review]
pr 4006 - update external/rabbitmq/test_* to pytest
Created attachment 8911741 [details] [review]
pr 4016 - update external/fs/test_* to pytest
Created attachment 8913227 [details] [review]
pr 4021 - last round of pytest updates

Comment 35

11 months ago
Commit pushed to master at https://github.com/mozilla-services/socorro

https://github.com/mozilla-services/socorro/commit/5d36f37d8ee45f03d5e58565731c15e198237e3d
bug 1361764 - pytest: the final chapter [no rush] (#4021)

* bug 1361764 - update external/postgresql/test_* to pytest

* bug 1361764 - update external/statsd/test_* to pytest

* bug 1361764 - update processor/* to pytest

* bug 1361764 - update everything else to pytest
The last item landed. Socorro proper no longer uses nose! Yay!

Follow up bug to convert the webapp to pytest is bug #1405675.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.