fix(test): strip comments before matching in the #44 and #117 sensors - #126
Merged
Merged
Conversation
tools/tests/test_runtime_lifecycle_invariants.py's regex-based source checks matched raw source text, so a commented-out call site with a decoy statement nearby still satisfied the check — mutation-tested on both test_shutdown_thread_never_touches_the_callback_registry (#44) and test_broadcaster_callbacks_record_their_owner_before_listening (#117). Add strip_comments(), the same //-and-block-comment-aware approach the justfile's N99 sensors already use, and run it over every extracted function body before the regexes see it. Fixes #123 Co-Authored-By: nevr-runtime <agents@sprock.io>
…raced_function Opus review of PR #126 found two remaining issues in the strip_comments fix: 1. False negative: closing a block comment appended nothing to the result, so `return/**/Unregister();` collapsed to `returnUnregister();` — gluing two real tokens together and breaking \bUnregister word-boundary matching for a call a comment merely interrupts, not one that is commented out. The compiler treats a block comment as whitespace; strip_comments now does too, appending a single space when the `*/` closes. 2. Scope gap: the #44/#117 sensors were wrapped in strip_comments() by hand, but every other regex/substring sensor in this file (e.g. test_every_boot_detour_result_is_consumed's `if (installed) return;` check) still matched raw, unstripped source — a commented-out real line with a decoy nearby still satisfied assertIn. Moved strip_comments() into extract_braced_function() itself so every sensor in the file gets the same comment-transparency the compiler has, and removed the now- redundant per-call wraps around the #44/#117 variables. Verified both fixes with a mutation suite (scratch copies under /var/tmp/work-nevr-runtime/issue123-mutation/): 6 real-regression mutations across the #44/#117 sensors still correctly fail pre- and post-fix: the return/**/Unregister(); false negative now correctly fails post-fix (missed pre-fix); and a new mutation against initialize.cpp's `if (installed) return;` (commented out with a decoy) now also correctly fails post-fix (missed pre-fix) — proving the scope-gap fix reaches a sensor the per-call wraps never touched. strip_comments edge cases (line comments, multi-line block comments, strings/char literals containing // or /*, a / b / c division) re-verified correct. just verify: green (CMAKE_BUILD_PARALLEL_LEVEL=4 just verify -> "verify: OK (mingw-release)"), including the file's own 87 ground-truth tests under test-auth-unit. Co-Authored-By: nevr-runtime <agents@sprock.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tools/tests/test_runtime_lifecycle_invariants.py's regex-based source checks matched raw source text, including comments — a commented-out call site with a decoy statement nearby still satisfied the check.strip_comments()(//and/* */aware, string/char-literal safe) and applies it to the extracted function bodies intest_shutdown_thread_never_touches_the_callback_registry(gameserver: shutdown thread calls Unregister(), which touches a callback registry documented as main-thread-only #44) andtest_broadcaster_callbacks_record_their_owner_before_listening(gameserver: 033b303 dropped cb.broadcasterOwner assignment, UnregisterBroadcasterCallbacks never unregisters anything #117), before any regex runs.justfile:468).Mutation test (manual, not committed)
EchoVR::Broadcaster* owner = GameServer::RecordBroadcasterOwner(*m_context);insrc/runtime/server/gameserver.cpp, replacing it withowner = nullptr;as a decoy → before the fix, the gameserver: 033b303 dropped cb.broadcasterOwner assignment, UnregisterBroadcasterCallbacks never unregisters anything #117 check still passed (confirmed by running the old regex withoutstrip_commentsagainst the mutated file); after the fix,test_broadcaster_callbacks_record_their_owner_before_listeningcorrectly FAILED.self->ShutdownUnregisterOnGameThread();in the shutdown-thread lambda →test_shutdown_thread_never_touches_the_callback_registrycorrectly FAILED.Test plan
python3 -m pytest tools/tests/test_runtime_lifecycle_invariants.py -v— 7/7 pass on the real treeCMAKE_BUILD_PARALLEL_LEVEL=4 just verify— green (verify: OK (mingw-release))Fixes #123
🤖 Generated with Claude Code