Repository navigation
Add support for test cases with more than 2 incremental runs - #3347
Conversation
Use .2, .3 etc. as the suffixes for files in the second and later runs.
| with open(mpath) as f: | ||
| files.append((join(base_path, fnam), f.read())) | ||
| elif p[i].id == 'stale': | ||
| elif re.match(r'stale[2-9]?$', p[i].id): |
There was a problem hiding this comment.
By convention we typically allow a redundant '1' (e.g. [out1] == [out]). (Or are you worried there would be multiple sections with the same pass number?)
There was a problem hiding this comment.
Ok, I'll accept stale etc. as well.
| if p[i].id == 'stale': | ||
| passnum = 1 | ||
| else: | ||
| passnum = int(p[i].id[-1]) |
There was a problem hiding this comment.
You're limiting this to 9 passes? :-)
| renamed_path = path[:-5] | ||
| if re.search(r'\.[2-9]$', path): | ||
| # Make sure new files introduced in the second and later runs are accounted for | ||
| renamed_path = path[:-2] |
There was a problem hiding this comment.
Oh, I see, this is why we can't have 10 or more passes...
There was a problem hiding this comment.
I'll update to allow more than 9 passes.
| if incremental == 0: | ||
| if incremental_step == 0: | ||
| # Not incremental | ||
| msg = 'Invalid type checker output ({}, line {})' |
There was a problem hiding this comment.
Maybe change "Invalid" to "Unexpected" in each branch? This wording has always confused me.
| self.check_module_equivalence( | ||
| 'rechecked', | ||
| testcase.expected_rechecked_modules, | ||
| 'rechecked' + suffix, |
There was a problem hiding this comment.
Adding the suffix makes for a slightly awkward error message, "Set of rechecked1 modules does not match expected set". Especially since this refers to the difference between rechecked[1] and rechecked[2].
There was a problem hiding this comment.
I'll update the message to be less confusing.
| m = re.search('# cmd: mypy -m ([a-zA-Z0-9_. ]+)$', program_text, flags=re.MULTILINE) | ||
| m2 = re.search('# cmd2: mypy -m ([a-zA-Z0-9_. ]+)$', program_text, flags=re.MULTILINE) | ||
| if m2 is not None and incremental == 2: | ||
| m2 = re.search('# cmd{}: mypy -m ([a-zA-Z0-9_. ]+)$'.format(incremental_step), |
There was a problem hiding this comment.
One would expect to be able to use "# cmd" with "# cmd2" and "# cmd3" and others together, but it seems you actually only support "# cmd" and one other. And you could spell the second one "# cmds" or "# cmdlol". Not sure if this is worth much deliberation but maybe it should match outN, staleN etc.?
There was a problem hiding this comment.
I think that this works correctly since we include the incremental step number in the regexp, so this lets you use # cmd2 and # cmd3 together, since the function is being called once per incremental step. The name m2 is kind of confusing though. I'll refactor this code a bit to make it clearer. # cmdlol is not accepted.
| -- Each test is run twice, once with a cold cache, once with a warm cache. | ||
| -- Before the tests are run the second time, any *.py.next files are copied to *.py. | ||
| -- Before the tests are run the second time, any *.py.2 files are copied to *.py | ||
| -- (and *.py.3 files for the third step, and so on). |
There was a problem hiding this comment.
A little awkwards wording, perhaps "Before the tests are run again, in step N any *.py.N files are ..." ? You might not need to "and *.py.3 etc." part then.
gvanrossum
left a comment
There was a problem hiding this comment.
You can merge it yourself after fixing the list() thing (and waiting for tests).
| expected: Optional[Set[str]], actual: Set[str]) -> None: | ||
| if expected is not None: | ||
| expected_normalized = list(sorted(expected)) | ||
| actual_normalized = list(sorted(actual.difference({"__main__"}))) |
There was a problem hiding this comment.
No need for list() around sorted().
Use .2, .3 etc. as the suffixes for files in the second and later runs.
I'll add additional multi-pass test cases in separate PRs.