Skip to content

test: document unchecked mock assertion arguments - #5049

Open
asukaminato0721 wants to merge 1 commit into
facebook:mainfrom
asukaminato0721:tests-only-pr-4795
Open

asukaminato0721 wants to merge 1 commit into
facebook:mainfrom
asukaminato0721:tests-only-pr-4795

Conversation

@asukaminato0721

@asukaminato0721 asukaminato0721 commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Mock assertion methods do not check arguments against the patched target signature. Bug-marked tests accept the current absence of diagnostics and identify the invalid arguments that should be reported. Passing control cases remain unmarked.

The testcase! regressions use bug = markers and comments documenting the correct behavior. These are passing tests of known bugs, following the repository test guidance.

The complete implementation and regression tests remain in PR #4795. Related issue: #3913.

Validation

Based on upstream main at ef6de648bce0dc6dcca0a21a9fbb7490f23ecce8.

  • cargo test -p pyrefly --lib -- test_assert_called_with_checks_target_signature test_patch_special_function_call_uses_target_signature test_patch_start_checks_target_signature test_async_patch_checks_await_assertion test_patch_bound_method_checks_bound_signature test_patch_new_callable_keeps_declared_result — passed.
  • python3 test.py --no-test --no-tensor-shapes --no-conformance --no-jsonschema --no-extension — passed.

@meta-codesync

meta-codesync Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

This pull request has been imported. If you are a Meta employee, you can view this in D122390435. (Because this pull request was imported automatically, there will not be any future comments.)

@codspeed

codspeed Bot commented Sep 29, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 18.46%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 1 regressed benchmark
✅ 34 untouched benchmarks
⏩ 9 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ invalidate_all 30.9 ms 37.9 ms -18.46%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing asukaminato0721:tests-only-pr-4795 (2374578) with main (18b7584)

Open in CodSpeed

Footnotes

  1. 9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@github-actions

This comment has been minimized.

@github-actions github-actions Bot added size/m and removed size/m labels Oct 2, 2026
@asukaminato0721 asukaminato0721 changed the title test: check mock assertion arguments against patched signatures test: document unchecked mock assertion arguments Oct 2, 2026
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

This branch has not been deployed

No deployments
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.

2 participants