Quiet plugin ring-buffer console echo during test runs#496
Merged
Conversation
A clean, all-green `test_run` printed thousands of alarming lines — ~500
`MCP | filler N` from the log-ring bound test, plus per-test dispatcher
`[recv]/[send]` chatter and `[error] ... malformed result` from negative-path
suites that deliberately drive failures. It made a passing run look broken.
`McpLogBuffer.log()` echoed every line to the Godot console unconditionally
(the existing `enabled` field was never consulted). Gate the console `print`
on `enabled and console_echo`; the ring buffer still records every line, so
tests that assert on buffer contents are unaffected. The test runner flips the
new static `console_echo` off for the duration of a run and restores it after,
so live logging is unchanged outside tests.
This does NOT touch engine-level C++ errors (JSON parse, get_node-from-outside-
tree, etc.) that negative-path tests trigger — those aren't routed through our
logger and still surface. The 33 raw `print("MCP | ...")` calls in the
update/lifecycle paths are also left as-is (real diagnostics, not buffer-routed);
trimming those is a possible follow-up.
Verified live on Godot 4.6.3: `MCP | filler` lines during a run dropped from
~500 to 0, dispatcher `[error]` lines from 2 to 0, total editor log from
thousands to ~400. GDScript suite: 0 failures (new `log_buffer` suite asserts
the ring still records under a muted echo). 0 parse errors on 4.6.3 and 4.3.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
Quiets the plugin's McpLogBuffer console echo during test runs so an all-green run isn't drowned in deliberately-induced ring-buffer fill messages and negative-path error chatter. The ring buffer continues to record every line, so content-based assertions are unaffected.
Changes:
- Add a static
console_echoflag onMcpLogBufferand gate theprint()inlog()on bothenabledandconsole_echo. - Test runner saves, mutes, and restores
console_echoaroundrun_suites(). - New
test_log_buffer.gdsuite verifying ring recording continues when echo is muted orenabledis false.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| plugin/addons/godot_ai/utils/log_buffer.gd | Adds static var console_echo; gates console print on enabled and console_echo. |
| plugin/addons/godot_ai/testing/test_runner.gd | Mutes McpLogBuffer.console_echo for duration of run_suites() and restores it. |
| test_project/tests/test_log_buffer.gd | New suite asserting ring recording continues under muted echo / disabled instance. |
| test_project/tests/test_log_buffer.gd.uid | UID file for the new test script. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Problem
A clean, all-green
test_runprinted thousands of alarming lines — making a passing run look broken. The bulk:MCP | filler Nlines from the log-ring bound test (test_dock),[recv]/[send]chatter, and[error] ... malformed resultlines from negative-path suites (e.g.test_dispatcher) that deliberately drive failures.Fix
McpLogBuffer.log()echoed every line to the Godot console unconditionally (the existingenabledfield was never consulted). Now:printis gated onenabled and console_echo. The ring buffer still records every line, so tests that assert on buffer contents viaget_recent()/total_logged()are unaffected.console_echo(defaulttrue, so production logging is unchanged) is flipped off by the test runner for the duration of a run and restored afterward.Scope / non-goals
get_node()from outside the active tree, etc.) that negative-path tests intentionally trigger are not routed through our logger and still surface — that's inherent and out of scope.print("MCP | ...")calls in the update/lifecycle paths are left as-is (real diagnostics, not buffer-routed). Trimming those is a possible follow-up.Verification
Live on Godot 4.6.3 (
test_runvia the live editor):[error]lineslog_buffersuite asserts the ring still records under a muted echo; a brittle "defaults true" test was intentionally omitted since the runner mutes the flag mid-run).🤖 Generated with Claude Code