Warn on incomplete concrete classes that inherit from abstract classes #7955
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pierre-Sassoulas
added
the
False Negative 🦋
sshane
changed the title
This comment has been minimized.
This comment has been minimized.
sshane
commented
Dec 22, 2022
This comment has been minimized.
This comment has been minimized.
sshane
force-pushed
the
W0223-widening
branch
from
0652845
to
855c354
Compare
This comment has been minimized.
This comment has been minimized.
|
Updated the test refs, but I'm not sure they're right. I'm getting lots of diffs for a lot of tests. Running pylint built from main on Windows 11 ( It appears I'm getting the correct output with Ubuntu 20.04, but |
for more information, see https://pre-commit.ci
|
Should be good now, let me know if there's any issues @DanielNoord! I had to add a print into the tester to get the results :/ |
|
For whatever reason, both Python 3.8 and 3.10 do not work to update the abstract method file. I'm running self = <pylint.testutils.functional.lint_module_output_update.LintModuleOutputUpdate object at 0x7fa1e3c6d690>
def runTest(self) -> None:
> self._runTest()
E AssertionError: Wrong results for file "abstract_method":
E
E Unexpected in testdata:
E 47: abstract-method
../pylint/testutils/lint_module_test.py:145: AssertionError
=========================================================================================================================================== short test summary info ===========================================================================================================================================
FAILED test_functional.py::test_functional[abstract_method] - AssertionError: Wrong results for file "abstract_method":
============================================================================================================================ 1 failed, 779 passed, 25 skipped in 62.76s (0:01:02) ============================================================================================================================= |
|
Could it be that the test simply fails? If haven't checked that myself locally |
|
Do you have any docs for what the comments do? For example, I'm not sure if Are the functional tests a sort of blend of replay with the file outputs and unit tests? |
This (attempts to) describe the functional test framework. Basically it tries to lint the file and sees where we expect messages to be raised and compares that against the actual output of the linting. |
|
@sshane This is quite close to being done. Do you have intention of working on it? Or shall we close it? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7955 +/- ##
==========================================
+ Coverage 95.82% 95.83% +0.01%
==========================================
Files 174 174
Lines 18810 18810
==========================================
+ Hits 18024 18026 +2
+ Misses 786 784 -2
|
This comment has been minimized.
This comment has been minimized.
|
@DanielNoord I'm having the same problem I did when I last worked on it, I verified my Running |
|
Narrowed it down to #9688. I don't have any more time to spend on this for the time being, feel free to get it across the line if it's simple for you, else you can close. |
This comment has been minimized.
This comment has been minimized.
|
I was expecting each line's warnings to be compared to the annotations, not one by one! A more clear error message would have been nice 🙂 |
This comment has been minimized.
This comment has been minimized.
|
@DanielNoord I added the count which would have surfaced this for me much quicker, let me know if it's something you want and I can split it out |
for more information, see https://pre-commit.ci
|
🤖 Effect of this PR on checked open source code: 🤖 Effect on home-assistant:
Effect on black:
Effect on pytest:
Effect on pandas:
Effect on sentry:
This comment was truncated because GitHub allows only 65536 characters in a comment. This comment was generated for commit 334651c |
DanielNoord
reviewed
Jun 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #7950
TODO: