Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Can't update functional test refs #9688

Open
sshane opened this issue Jun 3, 2024 · 5 comments · May be fixed by #9690
Open

Can't update functional test refs #9688

sshane opened this issue Jun 3, 2024 · 5 comments · May be fixed by #9690
Labels
Maintenance Discussion or action around maintaining pylint or the dev workflow Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@sshane
Copy link

sshane commented Jun 3, 2024 •
edited
Loading

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:

pylint/pylint/testutils/lint_module_test.py

Lines 256 to 261 in 86e1943

assert (
expected_messages == actual_messages
), self.error_msg_for_unequal_messages(
actual_messages, expected_messages, actual_output
)
self._check_output_text(expected_messages, expected_output, actual_output)

so this has a chance to run:

pylint/pylint/testutils/functional/lint_module_output_update.py

Lines 27 to 32 in 86e1943

def _check_output_text(
self,
_: MessageCounter,
expected_output: list[OutputLine],
actual_output: list[OutputLine],
) -> None:

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
@sshane sshane added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jun 3, 2024
@sshane sshane mentioned this issue Jun 3, 2024
Open
2 tasks
@Pierre-Sassoulas
Copy link
Member

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 :)

@Pierre-Sassoulas Pierre-Sassoulas added Maintenance Discussion or action around maintaining pylint or the dev workflow Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jun 4, 2024
@sshane
Copy link
Author

sshane commented Jun 4, 2024 •
edited
Loading

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.

@sshane sshane linked a pull request Jun 4, 2024 that will close this issue
Open
@Pierre-Sassoulas
Copy link
Member

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
Copy link
Author

sshane commented Jun 4, 2024

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

@Pierre-Sassoulas
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Maintenance Discussion or action around maintaining pylint or the dev workflow Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

functional tests: fix automatic ref updating sshane/pylint
2 participants
@Pierre-Sassoulas @sshane