Skip to content

Restore falsy attribute values on unpatch - #381

Open
kwy404 wants to merge 1 commit into
facebook:mainfrom
kwy404:fix-unpatch-falsy-values
Open

kwy404 wants to merge 1 commit into
facebook:mainfrom
kwy404:fix-unpatch-falsy-values

Conversation

@kwy404

@kwy404 kwy404 commented Sep 24, 2026

Copy link
Copy Markdown

What:

Unpatching now restores attributes whose original value was falsy.

Why:

The unpatchers in testslide/core/patch.py decided whether to restore or delete based on the truthiness of the saved value. Patching something like SomeClass.count = 0 or a module level FLAG = False with patch_attribute() deleted the attribute when the test finished, which then breaks every later access to it.

How:

Restore when restore is set (which patch_attribute() already computes from whether the attribute existed), keeping the existing value check so the mock_callable() callers behave as before.

Risks:

Low. The only behavior change is for attributes that existed with a falsy value.

Checklist:

  • Added tests, if you've added code that should be tested
  • Updated the documentation, if you've changed APIs (N/A)
  • Ensured the test suite passes
  • Made sure your code lints
  • Completed the Contributor License Agreement ("CLA")

I ran the *_testslide.py suites and the unittest suites locally on Python 3.12, plus ruff, flake8 and mypy. The new example in tests/patch_attribute_testslide.py fails before the change. mock_constructor_testslide.py has one failure on 3.12 (can call class methods) and cli_unittest needs termios, both the same on main.

The unpatchers in _patch only restored the original value when it was truthy and deleted the attribute otherwise, so patching an attribute whose value was 0, False, None or an empty string removed it once the test finished. Use the restore flag that patch_attribute already computes.
@meta-cla meta-cla Bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 24, 2026
@meta-codesync

meta-codesync Bot commented Sep 28, 2026

Copy link
Copy Markdown

@oxo42 has imported this pull request. If you are a Meta employee, you can view this in D122127123.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant