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.

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

gh-96661: Ensure default action for warnings is restored in the interpreter's state #96662

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ovchinnikov-v-a
Copy link

@ovchinnikov-v-a ovchinnikov-v-a commented Sep 7, 2022

On the first run all the tests from test_warnings suite pass.
On every subsequent run several tests fail because warnings being emitted via module.warn/module.warn_explicit functions are ignored by default and not caught by the catch_warnings class object.

It turns out that failures are caused by test_warnings._WarningsTests.test_default_action that changes default action for warnings to ignore but fails to restore it back to default.
Even though the code of test_warnings._WarningsTests.test_default_action contains the following finally block which sets module's default action member to its original value, this value is never propagated to the interpreter's state structure.

        finally:
            self.module.defaultaction = original

The quirk is in the broken reference chain.
At the very beginning of the test this chain looks perfectly correct: original -> module.defaultaction -> module._defaultaction -> interp.warnings.default_action.
However, test deletes module.defaultaction and assigns a different string literal (i.e. ignore) to it thus taking module.defaultaction away from the aforementioned chain:

                # Test removal.
                del self.module.defaultaction
                ...
                # Test setting.
                self.module.defaultaction = "ignore"

Furthermore, as soon as a warning is fired after module.defaultaction value has been updated, interp.warnings.default_action is set to reference new module.defaultaction.
The bottom line result is two separate reference chains: original -> module._defaultaction and module.defaultaction -> interp.warnings.default_action.
The assignment self.module.defaultaction = original in the test's finally block (see above) can by no means change interp.warnings.default_action value despite partially restoring the original reference chain: original -> module.defaultaction -> module._defaultaction.

The warnings C module interface is pretty terse containing only two functions: warn_explicit and _filters_mutated.
The latter one has nothing to do with interp.warnings.default_action value whereas the former does exactly what we need: sets interp.warnings.default_action to reference module.defaultaction.

…s state

A better way of changing the interpreter's state might exist out there.
@bedevere-bot
Copy link

bedevere-bot commented Sep 7, 2022

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@ovchinnikov-v-a ovchinnikov-v-a changed the title gh-96661: Ensure default action is restored in the interpreter's state gh-96661: Ensure default action for warnings is restored in the interpreter's state Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants