pylint-dev / pylint

It's not just a linter that annoys you!
https://pylint.readthedocs.io/en/latest/
GNU General Public License v2.0
5.33k stars 1.14k forks source link

Can't update functional test refs #9688

Open sshane opened 5 months ago

sshane commented 5 months ago

Bug description

The message counter is checked first before the output text, so we never have a chance to write the updated output file.

Need to flip these two statements:

https://github.com/pylint-dev/pylint/blob/86e194307affcb860244278f0c45d4717850601e/pylint/testutils/lint_module_test.py#L256-L261

so this has a chance to run:

https://github.com/pylint-dev/pylint/blob/86e194307affcb860244278f0c45d4717850601e/pylint/testutils/functional/lint_module_output_update.py#L27-L32

Configuration

No response

Command used

python tests/test_functional.py -k abstract --update-functional-output

Pylint output

Failure, depending on if you change the refs

Expected behavior

You can update the refs

Pylint version

pylint 3.3.0-dev0
astroid 3.2.2
Python 3.11.4 (main, Jul 20 2023, 22:46:38) [GCC 9.4.0]

OS / Environment

Ubuntu 20.04

Additional dependencies

No response

Pierre-Sassoulas commented 5 months ago

It's "by design", writing a new output file is useless (or misleading you into thinking it worked) if the functional test does not output the expected value. That said the functional test is still going to fail the line after, and you're only going to have a temporarily better output file while you fix the issue with the functional test. Feel free to open a MR to change the order :)

sshane commented 5 months ago

~Ah, maybe I'm confused about the relationship of the functional test and the output ref. I assumed that the output text file was a reference we compared the pylint output to, without any ignore tags. And the functional test was to assert certain warnings on certain lines.~

Nvm that's correct. pylint already outputs "here's what you need to copy/paste" in response to a failed test, so I don't believe fixing this would mislead you any more that it already does.

Pierre-Sassoulas commented 5 months ago

I assumed that the output text file was a reference we compared the pylint output to, without any ignore tags.

Yes, but I'm not sure if I understand the "without any ignore tags" part.

And the functional test was to assert certain warnings on certain lines.

Yes, or the absence of them, if you have unexpected warning the functional test file fail. (And if the functional test fail file updating the output is going to create a wrong output).

sshane commented 5 months ago

I just meant running pylint on the whole file, ignoring the warning annotations. Disregard.

Pierre-Sassoulas commented 5 months ago

It seems you're already deep in the truth/code so I'm not sure if this part of the doc might help (maybe to get a general idea): https://pylint.readthedocs.io/en/stable/development_guide/contributor_guide/tests/writing_test.html#functional-tests