fix: repair 4 failing tests and add exit-code checks for reservation CI (#176)#198
Merged
Merged
Conversation
63842ff to
bc51cd3
Compare
…CI (#176) - test_resource_trends.gd: Replace Globals.get_node with direct Main instance creation to work in headless mode (fixes 'Globals not declared' error) - test_food_upkeep.gd: Change extends TestSuite → extends SceneTree, wrap all tests in _initialize() (fixes 'Could not find base class TestSuite') - test_recruit_worker.gd: Add explicit type annotation for initial_count: int (fixes 'Cannot infer type' parse error) - game_state.gd: Add class_name GameState so it's accessible in strict mode (fixes 'Identifier not found: GameState' compile error) - .github/workflows/test.yml: Add exit-code capture and check for reservation tests step (was silently ignoring failures)
bc51cd3 to
ab58ef4
Compare
The 'class_name GameState' declaration conflicts with the autoload singleton named 'GameState' in project.godot. This caused a parse error: Class 'GameState' hides an autoload singleton. Remove class_name and use dynamic typing for the local variable that instantiates the autoload singleton.
There was a problem hiding this comment.
AI Automated Review (incremental)
Incremental review: reviewed the changes since the last managed review; unresolved findings from that review are carried forward.
Analysis engine: review@https://litellm.jory.dev/v1 (openai)
Recommendation
Approved. This PR addresses several critical issues in the test suite and CI pipeline, ensuring that failing tests are repaired and that CI failures are correctly captured rather than silently ignored.
Changes
Scripts
scripts/game_state.gd: Removedclass_name GameState. This is a necessary change to resolve an autoload conflict whereGameStatewas being treated as a global singleton in some contexts but needed to be instantiated manually in others (specifically for testing).scripts/main.gd: Updated_on_startup_new_gameto use untypedvar _gs = GameState.new()instead of explicit type annotationvar _gs: GameState = GameState.new(). This is a direct consequence of removing theclass_namefromgame_state.gdto fix the autoload conflict.
Tests
tests/test_recruit_worker.gd: Added explicit type annotationvar initial_count: int = main.state.workers.size()to satisfy GDScript v2 strict type inference requirements.tests/test_food_upkeep.gd(implied by PR description): Updated to extendSceneTreeand use_initialize()to align with the project's testing pattern.tests/test_resource_trends.gd(implied by PR description): Refactored to use a_make_main()helper instead of relying onGlobals.get_node("/root/Main"), which fails in headless mode.
CI/CD
.github/workflows/test.yml: Added exit-code capture (RES_EXIT=$?) and conditional checks for reservation tests. This ensures that if the Godot test command fails, the CI step correctly reports a failure instead of silently passing due to the subsequentcatcommand.
Linked Issue Fit
- Fixes PR 176: The PR successfully addresses the failing tests and CI issues described in the linked issue. The implementation follows the requirements to repair the standalone test scripts and enforce exit codes in the workflow.
Standards Compliance
- GDScript Conventions: The PR correctly addresses strict type inference issues in
test_recruit_worker.gdand resolves theclass_nameconflict ingame_state.gdwhich was causing compilation errors in the test environment. - Testing Pattern: The changes to
test_food_upkeep.gdalign the test structure with existing patterns intest_runner.gdand other test files in the repository.
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.
Fixes #176
This PR repairs all 4 failing standalone test scripts and adds exit-code enforcement for reservation tests in CI.
Changes
test_resource_trends.gd: Replace
Globals.get_node("/root/Main")with direct Main instance creation via a_make_main()helper function. This eliminates the parse error 'Identifier Globals not declared' that occurs in headless mode since there's no autoload scene tree.test_food_upkeep.gd: Change
extends TestSuite→extends SceneTreeand wrap all tests inside_initialize(). TestSuite is not a built-in Godot class; the pattern now matches test_runner.gd, test_recruit_worker.gd, and test_worker_cap.gd.test_recruit_worker.gd: Add explicit type annotation
var initial_count: int = main.state.workers.size()to satisfy GDScript v2 strict type inference (fixes 'Cannot infer the type of initial_count variable').game_state.gd: Add
class_name GameStateat the top so the class is globally accessible in strict mode. This fixes the compile error 'Identifier not found: GameState' that occurs when test_worker_cap.gd preloads main.gd (which uses GameState as a type annotation)..github/workflows/test.yml: Add exit-code capture (
RES_EXIT=$?) and conditional check for the reservation tests step. Previously failures were silently ignored since onlycatwas run after the Godot command.