[Enesuygurs]

I have made things!

Checklist

• I have double checked that there are no unrelated changes in this pull request (old patches, accidental config files, etc)
• I have created at least one test case for the changes I have made
• I have updated the documentation for the changes I have made
• I have added my changes to the CHANGELOG.md

Related issues

• Fixes assert_trace does not work correctly with functions that use the same decorator #876

assert_trace compared the __code__ of the function being searched for with the code object of each frame on the stack. Every wrapper produced by one decorator shares the same code object, so assert_trace(Success, func_one) passed when the container actually came from func_two.

Unwrapping to the inner function, the approach tried in the issue, breaks @safe: the Failure is created in the wrapper after the inner function has already returned. Instead, when the code object matches, this change also checks that the frame's closure variables are the same objects as the searched function's closure cells. Each wrapper closes over its own wrapped function, so the wrappers can be told apart, and @safe keeps working. This works with and without functools.wraps. Closure cells that are still empty are skipped.

Added tests: a function wrapped by a shared decorator is still found, and a different function with the same decorator no longer passes the check. The second test fails on master and passes with this change. make unit passes, and so do ruff, flake8 and mypy.

[codspeed]

Merging this PR will not alter performance

1 note

ℹ️ 2 benchmarks have no flamegraph

The flamegraph wasn't generated: the symbols of the profiling trace could not be resolved, because no information about the language frames was collected.

See affected benchmarks

✅ 22 untouched benchmarks

---

_{Comparing Enesuygurs:fix/876-assert-trace-same-decorator (f881784) with master (2ffd685)}

[sobolevn]

please, add a test case where you compare objects that trigger an exception on __eq__

[sobolevn]

can we have a fast path here? what if they are not? comparing everything is slow and can cause different side-effects and other problems.

[Enesuygurs]

Thanks for the review! I added test_identity_closure_assert_trace and test_identity_closure_failed_assert_trace. In both, the decorator closes over an object whose __eq__, __hash__ and __bool__ all raise, and assert_trace still finds the right function and rejects the other one without calling any of them. The closure check only compares by identity (is).

[sobolevn]

what about the fast path? :)

[Enesuygurs]

Good point! I added a fast path: if the searched function has no closure, a matching code object is enough, so we return right away without reading the frame's locals. The closure comparison now only runs for wrappers that share a code object, which is exactly the decorator case.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (82ef3ef) to head (f881784).
⚠️ Report is 649 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##            master     #2552    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           80        82     +2     
  Lines         2485      2692   +207     
  Branches       437        44   -393     
==========================================
+ Hits          2485      2692   +207     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sobolevn]

Thanks!