Check documentation examples for fatal errors #6475
Conversation
jacobtylerwalls
added
the
Maintenance
Pull Request Test Coverage Report for Build 2250189067
💛 - Coveralls |
| actual = "test_result" | ||
| self.assertEqual(actual, "expected") |
There was a problem hiding this comment.
| actual = "test_result" | |
| self.assertEqual(actual, "expected") | |
| # Nothing, as an assert of a string literal will always pass | |
| pass |
There was a problem hiding this comment.
I guess this is just taste and could go either way, but I was figuring the most likely mistake a beginner makes is not "oh I don't need to assert in the first place" but "oops I was trying to assert a comparison but I forgot to put the right hand side of it". So the solution is to put in the right hand side.
There was a problem hiding this comment.
Make sense, Daniel also think this example was better so we kept it in the follow-up MR :)
| actual = "test" | ||
| wanted = "Directly asserting a string literal will always pass" | ||
| assert actual == wanted |
There was a problem hiding this comment.
| actual = "test" | |
| wanted = "Directly asserting a string literal will always pass" | |
| assert actual == wanted | |
| # Nothing, as an assert of a string literal will always pass | |
| pass |
| @@ -91,7 +91,7 @@ def __init__(self, test_file: Tuple[str, Path]) -> None: | |||
| args_list=[ | |||
| str(test_file[1]), | |||
| "--disable=all", | |||
| f"--enable={test_file[0]}", | |||
| f"--enable={test_file[0]},astroid-error,fatal,syntax-error", | |||
There was a problem hiding this comment.
| f"--enable={test_file[0]},astroid-error,fatal,syntax-error", | |
| f"--enable={test_file[0]},F,E", |
There was a problem hiding this comment.
Ah, I don't think we want to enable all errors, though. Just the fatal ones from pylint/astroid breaking, not antipatterns in the code under test.
There was a problem hiding this comment.
What would be an example of error we want to keep when we're not specifically documenting it ?
There was a problem hiding this comment.
Good question. I thought I remembered the original discussion was "don't try to pass every pylint check" but you have a point that maybe errors are worth avoiding. Things like undefined-variable can be tedious to avoid if you have initialize all your thing and my_list etc. Let me see what fails if we add this.
There was a problem hiding this comment.
some examples (there are more):
doc/data/messages/w/wrong-import-order/good.py:6:0: E0611: No name 'utils' in module '' (no-name-in-module)
doc/data/messages/y/yield-inside-async-function/good.py:6:0: E0102: function already defined line 1 (function-redefined)
There was a problem hiding this comment.
Hmm yeah wrong-import-order is possible to do if we remove the "our own lib" part but then it would be incomplete. For the other one we might need to find a way to propose multiple solution to the same bad.py but this is a non trivial refactor to do. I was thinking generally that being forced to create example without error would make the examples better but let's revisit that later when the cost to do it is lower.
There was a problem hiding this comment.
The boundary is probably subjective. I asked a contributor to fix an easy TypeError from not passing an argument to a function in #6472
There was a problem hiding this comment.
We could also explicitly disable the errors in the example ? (# pylint: disable=no-name-in-module for example) Would prevent us from not seeing that there's an issue in an example.
We missed the syntax error fixed in 98996bd because we were disabling fatal errors when checking "good.py" in the message examples.
I also noticed
syntax-erroris slightly different fromastroid-errorso added it to the messages added in #6365