%b-%Y-%d by jbrockmendel · Pull Request #441 · dateutil/dateutil

jbrockmendel · 2017-08-09T03:52:04Z

Apparently all the cool kids are transitioning to pytest. To get a feel for it I took a few of the more easily-parametrized cases from test_parser.py and implemented them in test_parser_parametrized.py. Explicitly looping over all the valid formats exposed a handful that are not currently handled correctly (and clearly, not currently tested). Namely:

from dateutil import parser
>>> parser.parse('Sept 2003 25')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/site-packages/dateutil/parser.py", line 1182, in parse
    return DEFAULTPARSER.parse(timestr, **kwargs)
  File "/usr/local/lib/python2.7/site-packages/dateutil/parser.py", line 581, in parse
    ret = default.replace(**repl)
ValueError: day is out of range for month

This happens because resolve_ymd never considers the possibility (month, year, day). This PR adds that particular check.

… parametrized tests Adjust tox.ini and .gitignore accordingly

…parametrized Fix resolve_ymd for case that fails under new tests

pganssle · 2017-08-09T17:45:08Z

@jbrockmendel I would say that there's no reason to break test_parser into two files. I've been meaning to switch over to parameterized tests for a while now - I've been doing something like that in a half-assed way until I got around to actually adding the dependency.

Anyway, the CI is broken because I think you need to add a tests_require line into the setup.py.

…res pytest

pganssle · 2017-08-10T05:59:20Z

+token_opts = [['%Y'], ['%b', '%B'], ['%d', '%-d']]
+
+prods = itertools.product(*token_opts)
+perms = [y for x in prods for y in itertools.permutations(x)]


Not sure if this is more readable per se, but I believe this is equivalent to:

list(itertools.chain.from_iterable(map(itertools.permutations, itertools.prod(*token_opts))))

or possibly better:

list(itertools.chain(*(itertools.permutations(x) for x in itertools.prod(*token_opts))))

I find the two-line version more readable, but am pretty close to indifferent on this.

jbrockmendel · 2017-08-10T16:15:46Z

pytest doesn't allow for yield-based tests, right? I tried having this test run for (much) more than just 2003-09-25 and it looks like it created the tests as a list rather than a generator.

pganssle · 2017-08-14T21:45:07Z

I can't reproduce this test failure on my local machine with Python 3.4 on Linux. Is this a travis thing?

jbrockmendel · 2017-08-16T05:15:21Z

The Travis errors look like they are coming from sys.modules.values() changing size in iteration. Is this showing up elsewhere? If not, I'd guess that pytest isn't playing nicely with... something in py 3.2, 3.3, 3.4.

pganssle · 2017-08-16T06:26:14Z

Yeah, that was what I figured, but I'm not sure what because my machine doesn't do it when I spin up a Python 3.4 conda environment. I was thinking I'd maybe try removing the profiling stuff, then the coverage stuff sequentially and see if either of those fixes it, at least to narrow it down.

jbrockmendel · 2017-08-26T02:43:01Z

I just pushed a commit that uses regular nose-style tests instead of pytest. We'll see if that was the issue with the Travis error.

pganssle · 2017-08-26T13:33:13Z

@jbrockmendel What was it you changed that fixed this? I just found out about debug mode on TravisCI, so I was thinking I'd try and debug it that way.

jbrockmendel · 2017-08-26T15:35:22Z

I just removed pytest and changed the new test to yield nose-style.

pganssle · 2017-09-01T17:39:45Z

@jbrockmendel I'll try and look at this in more detail this weekend, I'm not sure I understand how these yield tests work, and it seems like you are still actually using pytest here. You didn't squash the old broken commit, right? I'm going to try and figure out why it was broken in the first place.

jbrockmendel · 2017-09-03T18:35:44Z

I think you're right that I didn't completely remove pytest. I can make a new PR that just fixes the bug and adds the tests, completely stripping out the other changes. Let me know your preference.

jbrockmendel · 2017-10-25T17:36:11Z

I'd like to wrap up at least the bug-fix part of this. Is the pytest problem still a problem? If so, I'll make a new PR that uses the older test setup.

pganssle · 2017-10-25T19:00:03Z

This is the most infuriating thing. I managed to open an interactive shell, but I still can't replicate the problem.

Anyway, yeah, I think just drop both all the pytest stuff including the yield tests. Switch that from using assert to using self.assertEqual. Normally I don't like a single test that handles a bunch of test cases, but as a temporary matter before we switch over to pytest and properly parametrized tests, we can just generate the tests inside the test function.

pganssle · 2017-10-27T18:08:40Z

Superceded by #481

jbrockmendel added 3 commits August 8, 2017 20:17

Use pytest for parameterized tests, implement basic ISO test cases as…

2abc9e6

… parametrized tests Adjust tox.ini and .gitignore accordingly

Remove form test_parser.py tests that are now handled by test_parser_…

1587de0

…parametrized Fix resolve_ymd for case that fails under new tests

Simpler fix for resolve_ymd; add coveragerc

0e02a21

jbrockmendel added 2 commits August 9, 2017 10:54

Merge test_parser_parametrized back into test_parser, add tests_requi…

04f35b9

…res pytest

Add missing import itertools

dbaa381

pganssle reviewed Aug 10, 2017

View reviewed changes

Revert from pytest in hopes avoid fixing travis problem

bd34f51

Merge branch 'master' of https://github.com/dateutil/dateutil into b_Y_d

162f2e9

jbrockmendel force-pushed the b_Y_d branch from cf573f4 to 162f2e9 Compare October 25, 2017 17:58

jbrockmendel mentioned this pull request Oct 26, 2017

Add fix for b-Y-d cases; add tests; remove redundant tests #481

Merged

pganssle closed this Oct 27, 2017

pganssle added this to the wontfix milestone Oct 27, 2017

pganssle added bug tests parser labels Oct 27, 2017

Uh oh!

Conversation

jbrockmendel commented Aug 9, 2017

Uh oh!

pganssle commented Aug 9, 2017

Uh oh!

pganssle Aug 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jbrockmendel Aug 10, 2017

Choose a reason for hiding this comment

Uh oh!

jbrockmendel commented Aug 10, 2017

Uh oh!

pganssle commented Aug 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jbrockmendel commented Aug 16, 2017

Uh oh!

pganssle commented Aug 16, 2017

Uh oh!

jbrockmendel commented Aug 26, 2017

Uh oh!

pganssle commented Aug 26, 2017

Uh oh!

jbrockmendel commented Aug 26, 2017

Uh oh!

pganssle commented Sep 1, 2017

Uh oh!

jbrockmendel commented Sep 3, 2017

Uh oh!

jbrockmendel commented Oct 25, 2017

Uh oh!

pganssle commented Oct 25, 2017

Uh oh!

pganssle commented Oct 27, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pganssle Aug 10, 2017 •

edited

Loading

pganssle commented Aug 14, 2017 •

edited

Loading