-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Modify incremental tests to allow initial error #2069
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
Modify incremental tests to allow initial error #2069
Conversation
This commit modifies the testing framework to allow us to specify the expected output during both the first and second runs of incremental mode, instead of mandating that the first run typechecks. This is useful since it lets us test to make sure incremental recovers correctly in the presence of errors.
This commit replaces the [out] sections with [out1] and [out2] in incremental tests.
This commit adds two basic tests to make sure that an initial error and differing errors in both runs of incremental is checked correctly.
This commit updates description at the top of check-incremental.test to explain how the [out1], [out2], [rechecked], and [stale] sections now work.
-- should be annotated in the [stale] annotation. Note that a file that ends up | ||
-- producing an error does not create a new cache file and so is not considered stale. | ||
-- | ||
-- The test suite will automatically assume that __main__ is automatically stale |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
automatically ... automatically
Thanks! I know I've been wanting this. I'm not sure it's a good idea to require [out1] and [out2] sections for all incremental tests though. How about if an incremental test has only an [out] section that means [out1] is empty and [out2] is set to the [out] section? Then the size of the diff would be much smaller. Or do you think it's useful to remind everyone about the two runs? I'm 50/50 so if you've laready thought about it I defer to your consideration. |
This commit makes the [out1] and [out2] sections optional. If omitted, it is expected there are no errors on that particular run.
After some reflection, I've decided that I'll make the [out1] and [out2] sections optional -- if either is omitted, it means we expect that specific run to output no errors. However, I don't know if I want to encourage the use of [out] in incremental tests -- I think it's potentially ambiguous what it means, and I think explicitly specifying whether you mean [out1] or [out2] would be more readable. (If you disagree, I'm happy to change this back -- I don't actually care that strongly about this issue.) So basically, the test cases are now a bit shorter/less boilerplate-y, but the diffs aren't any less noisy, unfortunately. |
That looks like a good improvement! |
This pull request modifies the testing framework to allow us to specify the expected output during both the first and second runs of incremental mode, instead of mandating that the first run typechecks.
This is useful since it lets us write tests that check whether or not incremental recovers correctly in the presence of errors.