-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Add new error code tests #34051
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
Add new error code tests #34051
Conversation
Can you describe the purpose of these tests? It seems to me the one thing they are validating is that the reported error does not change from one error to another. That is, perhaps the subsystem in the compiler that produces a specific error might change, resulting in a different error message for the same error - this prevents that. But I'm not sure that is likely to happen or that it is particularly undesirable. Furthermore, if that specific check is desirable - preventing a particular semantic error from changing from one error code to another - we can do it in a much more comprehensive way automatically. For example, compiletest could record all the errors it sees in one run of the test suite, then revalidate them in the future. That would prevent error code drift for all errors at once. |
Partially, yes. However, the main purpose of these tests is more bound to the errors explanation: a lot of them are completely outdated. So I'm hoping that if someone breaks one of these tests, he/she will update the corresponding error explanation as well.
It's not, like said above. Just forcing people to update code linked to their changes. sweet dream :-) |
Are these errors not captured in existing tests? Presumably if they break these they will also break any other tests that are testing these errors. If they are not captured by other tests, then why not write them like the other compile-fail tests, using the error message instead of the code, and naming the tests according to what is being tested? On that note it may be a better practice to write compile-fail tests by just writing the error codes instead of snippets of the error text, but it's not obvious to me. This PR looks to be establishing a new pattern for testing compile-fail tests, but if we are going to do that then perhaps there should be wider discussion and acknowledgement. |
Generally, that's not a problem if the error message change. The problem here is in case that the error message change for the same code (and it happened quite a lot) or if the error message get removed (which happened also quite a lot). |
@brson - With the error work we're doing now, some of the error messages are getting reworded, which will probably make them harder to google. Unfortunate, but maybe we can still use the error codes so that they can search for a solution. If both the message and the code drift, I'm not sure what they'd use to look up a solution. |
It's still unclear to me what the intent is here. Earlier @GuillaumeGomez stated "However, the main purpose of these tests is more bound to the errors explanation: a lot of them are completely outdated. So I'm hoping that if someone breaks one of these tests, he/she will update the corresponding error explanation as well." And I still don't see how these tests accomplish that any differently than existing tests, though I admit I may not have understood @GuillaumeGomez's follow up comment. But we're also talking about preventing error code 'drift', which is the thing that this appears to me to accomplish. And if that is the goal, then there are better strategies than writing new tests for every error by hand. @GuillaumeGomez proposed one earlier on IRC that I have forgotten the details of (maybe you can restate it here). |
I'll restate some of my earlier questions that I don't feel have been answered. Do these specific test provide coverage of errors that don't exist? Is this intended to be the new way to write compile-fail tests? Should all compile-fail tests be written like this? |
The scheme @GuillaumeGomez proposed on IRC for preventing drift is coming back to me: we would modify the test runner for the error code descriptions (which contain test cases for all errors) to verify that it saw the correct error code in each test. |
@brson: My point is the following one: sometime, a same code will not return the same error code. If the error code's text change on the stderr, it doesn't really matter. However, the problem is if the same code doesn't return the same error code: this is this point I want to check. |
This PR can now be closed. |
r? @steveklabnik
cc @jonathandturner