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

use setup_class instead of setUp #735

Merged
merged 1 commit into from Feb 26, 2019
Merged

Conversation

jbrockmendel
Copy link
Contributor

Most of the parser tests are written with the older unittest structure. Now that we're using pytest, some of that can be cleaned up.

The only substantive change is using a classmethod setup_class instead of a regular method setUp, so it only gets called once instead of for each test. Small-n it looks like this saves 20-30% of the runtime for that file (which is only 3-5 seconds, so really not a big deal).

There are a lot of self.assertEqual statements so getting them all will take a few passes.

@pganssle pganssle closed this Feb 22, 2019
@pganssle pganssle reopened this Feb 22, 2019
@pganssle
Copy link
Member

@jbrockmendel Sorry, I let this sit unreviewed for too long and now it's got some serious merge conflicts. Do you want to resolve them or should I?

@jbrockmendel
Copy link
Contributor Author

I'm happy to resolve them, but it'll be a few days before I can get around to it.

@pganssle
Copy link
Member

No rush.

@jbrockmendel
Copy link
Contributor Author

This should be ready. I've identified some more test cleanups to do in another pass, will wait until this is merged.

@@ -152,7 +152,7 @@ def test_parser_default(parsable_text, expected_datetime, assertion_message):
assert parse(parsable_text, default=datetime(2003, 9, 25)) == expected_datetime, assertion_message


class TestFormat(unittest.TestCase):
class TestFormat(object):
Copy link
Member

Choose a reason for hiding this comment

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

I've been writing so much Python 3 lately that I was initially taken aback by this. Sooooooon.

@pganssle pganssle merged commit fdc91e1 into dateutil:master Feb 26, 2019
@pganssle
Copy link
Member

Gah, forgot a news entry. No need to make a separate PR for it considering it's an internal refactoring, I'll just do it later.

pganssle added a commit to jbrockmendel/dateutil that referenced this pull request Feb 26, 2019
This is merged in the branch for PR dateutil#882 because PR dateutil#735 was
accidentally merged without a changelog.
@jbrockmendel jbrockmendel deleted the cmethod branch February 26, 2019 23:26
@pganssle pganssle mentioned this pull request Nov 3, 2019
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.

None yet

2 participants