-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Makes functional tests auto-update works even if the current output is wrong #6891
Makes functional tests auto-update works even if the current output is wrong #6891
Conversation
8f4b536
to
437acf5
Compare
Pull Request Test Coverage Report for Build 2486769284
💛 - Coveralls |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really understand the use case.
If the test fails how can we know what the expected output is?
You're rebasing a branch with change in the functional expected output. You don't want to fix the merge conflict, you just want to regenerate with the update option. Right now the update option fail if the output contain |
Okay, but simply discarding the merge change also works right? With the new code we only raise an |
Sure, but it's more work, and the message telling us to use update is wrong.
The functional test will also fail and ask you to update because we return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The functional test will also fail and ask you to update because we return
cls("", 0, 0, None, None, "", "", "")
which will never be valid.
👍
But let's add a changelog entry in the Internal changes
section.
…ent output is wrong Previously if an exception was raised during parsing we re-raised an explanation. Most of the time I just want to regenerate the output from the current pylint's output. It's what the exception suggested but it's impossible to do as the error is also raised when updating, so you had to empty the file.
437acf5
to
2d9688f
Compare
doc/whatsnew/2/2.15/index.rst
Outdated
@@ -61,3 +61,8 @@ Internal changes | |||
* ``pylint.testutils.primer`` is now a private API. | |||
|
|||
Refs #6905 | |||
|
|||
* Fixed an issue where it was impossible to regenerate functional tests output when they existing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Fixed an issue where it was impossible to regenerate functional tests output when they existing | |
* Fixed an issue where it was impossible to regenerate functional tests output when the existing |
2d9688f
to
597f6bf
Compare
* Fixed an issue where it was impossible to update functional tests output when the existing | ||
output was impossible to parse. Instead of raising an error we raise a warning message and | ||
let the functional test fail with a default value. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
This is the PR right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We never put PR numbers in the changelog right? If there isn't an issue we just leave that line empty.
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Daniël van Noord <[email protected]>
🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉 This comment was generated for commit 02998c4 |
Type of Changes
Description
Previously if an exception was raised during parsing we re-raised an explanation.
Most of the time I just want to regenerate the output from the current pylint's
output. It's what the exception suggested but it's impossible to do as the error
is also raised when updating, so you had to empty the file.