Skip to content

fix: only count user-code frames toward max_stack_depth#513

Closed
irajaryan wants to merge 1 commit intoboxed:mainfrom
irajaryan:fix/378-user-frames-only-max-stack-depth
Closed

fix: only count user-code frames toward max_stack_depth#513
irajaryan wants to merge 1 commit intoboxed:mainfrom
irajaryan:fix/378-user-frames-only-max-stack-depth

Conversation

@irajaryan
Copy link
Copy Markdown
Contributor

Closes #378.

record_trampoline_hit previously decremented the max_stack_depth budget for every frame walked, including its own frame, the injected _mutmut_trampoline, and any third-party / stdlib frames. This meant a configured value of N didn't correspond to anything a user could reason about.

This change only counts frames whose file lives under the mutants/ working directory and whose function name is not _mutmut_trampoline. os.path.realpath is used so the comparison works on macOS where /var is a symlink to /private/var.

The e2e config project's max_stack_depth is updated from 8 to 5 to keep the same coverage pattern under the new counting; the snapshot is unchanged.

Six unit tests cover: budget disabled, within/over budget, third-party frames excluded, trampoline frames excluded, and the existing test-runner heuristic preserved.

@Otto-AA
Copy link
Copy Markdown
Collaborator

Otto-AA commented May 3, 2026

Thanks for the PR, I didn't see it and worked on this simultaneously together with some other changes in #514 . There I decided to only include stack frames part of the source_paths config, so test setup is also not counted.

os.path.realpath is used so the comparison works on macOS where /var is a symlink to /private/var.

That's something I did not think about in my implementation. Resolving symlinks could indeed be necessary.

@irajaryan
Copy link
Copy Markdown
Contributor Author

Thanks for the PR, I didn't see it and worked on this simultaneously together with some other changes in #514 .

Ah, didn't see #514, looks like a much bigger cleanup and I think filtering against source_paths is the right call (it correctly excludes test files, which my approach doesn't).

Happy to close this in favor of #514. Let me know if I need to open a follow-up PR with just the realpath fix once #514 lands, or if you'd prefer to fold it into #514 directly. The relevant bit is small.

Otto-AA added a commit that referenced this pull request May 9, 2026
The PR #513 used os.path.realpath for this.
I think pathlib.Path.resolve() should also work fine,
and we can keep using pathlib.Path.

Using strict=True, as all files/directories should exist.
Otto-AA added a commit that referenced this pull request May 9, 2026
The PR #513 used os.path.realpath for this.
I think pathlib.Path.resolve() should also work fine,
and we can keep using pathlib.Path.

Using strict=True, as all files/directories should exist.
Otto-AA added a commit that referenced this pull request May 9, 2026
The PR #513 used os.path.realpath for this.
I think pathlib.Path.resolve() should also work fine,
and we can keep using pathlib.Path.

Using strict=True, as all files/directories should exist.
@Otto-AA
Copy link
Copy Markdown
Collaborator

Otto-AA commented May 9, 2026

I've changed my implementation to use Path.resolve, which also resolves symlinks. I think it should cover the same use cases as os.path.realpath. If there's something not working, please let me know or create a PR.

And thank you for the PR nonetheless! I'm still surprised that we both decided at the same time to fix this, after the issue has been open for a year already

@Otto-AA Otto-AA closed this May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Only count user-code frames for max_stack_depth

2 participants