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

Introduce parameterized pytest suite #91

Closed
wants to merge 1 commit into from
Closed

Introduce parameterized pytest suite #91

wants to merge 1 commit into from

Conversation

vaneseltine
Copy link
Contributor

Commit message

This creates an alternative test suite using the wonderful pytest
and parameterizing over 400 names off into json storage. It is
intended to include all functionality of the existing test suite,
while also:

  • Segmenting most names into json format in ./pytest/names
  • Keeping exactly the same groupings by class (though tweaked names)
  • Keeping all test names visible
  • Folding several comments into explicit expected fails and skips
  • Improving visibility of such xfails and skips
  • Automagically running twice for CONSTANTS.empty_attribute_default
  • Adding no further requirements beyond the pytest package

A note on Travis

This build will fail because Travis appears to have dropped support or moved archives for Pythons 2.6, 3.2, and 3.3. E.g. just now:

3.3 is not installed; attempting download
Downloading archive: https://storage.googleapis.com/travis-ci-language-archives/python/binaries/ubuntu/16.04/x86_64/python-3.3.tar.bz2
0.12s$ curl -sSf -o python-3.3.tar.bz2 ${archive_url}
curl: (22) The requested URL returned error: 404 Not Found
Unable to download 3.3 archive. The archive may not exist. Please consider a different version.

These tests are passing on every Python version that Travis is successfully downloading.

Pytest

Most of the work here was simply reformatting tests into pytest, then refactoring out the data from repeated test functions into json files. There are a few other changes here and there, but I tried not to add too many unnecessary changes.

For those unfamiliar with pytest, as a simple example of the transformation take these two:

class SuperSimpleNamesHandlingTests(HumanNameTestBase):

    def test_first_name(self):
        hn = HumanName("Larry Boberry")
        self.m(hn.first, "Larry", hn)
        self.m(hn.last, "Boberry", hn)

    def test_assume_title_and_one_other_name_is_last_name(self):
        hn = HumanName("Gary Nanafanafoferry")
        self.m(hn.first, "Gary", hn)
        self.m(hn.last, "Nanafanafoferry", hn)

Translated into pytest -- with little more than a regex substitution -- they become:

class TestSuperSimpleNamesHandling:

    def test_first_name(self):
        hn = HumanName("Larry Boberry")
        assert hn.first == "Larry"
        assert hn.last == "Boberry"

    def test_assume_title_and_one_other_name_is_last_name(self):
        hn = HumanName("Gary Nanafanafoferry")
        assert hn.first == "Gary"
        assert hn.last == "Nanafanafoferry"

Which is fine, but the true special sauce to actually improve the test suite is using pytest to transform a sequence of things being run in a test into a sequence of identical tests run on each thing:

class TestSuperSimpleNamesHandling:
    names_to_test = [
        ("Larry Boberry", "Larry", "Boberry"),
        ("Gary Nanafanafoferry", "Gary", "Nanafanafoferry")
    ]

    @pytest.mark.parametrize("raw, first, last", names_to_test)
    def test_first_name(self, raw, first, last):
        hn = HumanName(raw)
        assert hn.first == first
        assert hn.last == last

And now it's incredibly easy to add a hundred more. Anywhere there was a loop inside a test to make repeat assertions, there can instead be a series of tests.

Additional notes

I read through the discussion of the 2017 proposal at #61 before working this through. I believe that this overhaul will satisfy the stated desire for a more nuanced test strategy without the downsides identified in the previous approach.

I didn't want to change support, so allowing Python 2.6 (!) means pinning to pytest v3.3. They dropped support for Python 2.7 and 3.4 with pytest v4.6 (see https://docs.pytest.org/en/latest/py27-py34-deprecation.html).

This is pretty substantial, so of course I'm expecting to have some discussion and changes.

This creates an alternative test suite using the wonderful pytest
and parameterizing over 400 names off into json storage. It is
intended to include all functionality of the existing test suite,
while also:

 * Segmenting most names into json format in ./pytest/names

 * Keeping exactly the same groupings by class (though tweaked names)

 * Keeping all test names visible

 * Folding several comments into explicit expected fails and skips

 * Improving visibility of such xfails and skips

 * Automagically running twice for CONSTANTS.empty_attribute_default

 * Adding no further requirements beyond the pytest package
@vaneseltine
Copy link
Contributor Author

Oh, this does not duplicate the ability of ./tests.py to parse a single test string. This version passes through any command line arguments to pytest.

I'd honestly recommend just spinning that off into a separate script. I can write that as a separate PR if desired.

@vaneseltine
Copy link
Contributor Author

I'm going to withdraw this PR. I ended up spending more time in this area and spun off my own version (nominally). This PR has a couple of minor issues, but overall should be a useful reference if you wanted to move nameparser to pytest.

@derek73
Copy link
Owner

derek73 commented Dec 12, 2019

Thanks for the pull request. I got pulled away from this project for a while but finally got to take a closer look. I like the format, seems to extract everything we need into a more flexible json format and finally break it up into separate files. Definitely appreciate that it include the id string so there's some indication of what it was meant to test. I haven't checked it out and run the tests myself yet but I may still do that and end up merging it. Definitely looks like a better place to start than zero and a much better structure than currently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants