Skip to content

Add tests pinning console output and removal quirks - #71

Merged
dmccoystephenson merged 1 commit into
masterfrom
feature/pin-console-output-and-removal-quirks
Nov 4, 2026
Merged

dmccoystephenson merged 1 commit into
masterfrom
feature/pin-console-output-and-removal-quirks

Conversation

@dmccoystephenson

Copy link
Copy Markdown
Member

Summary

Stage B (unit-test expansion) cycle. The open-issue backlog was empty at triage, and Environment::printConsoleRepresentation() was the one public behaviour-bearing method with no test exercising it. No production code is changed.

  • Tests 60–61 pin printConsoleRepresentation(): the row-by-row layout, the [x] and [ ] cells, and the misalignment after setSize that the README already describes.
  • Test 62 pins getFirstEntity following location order rather than insertion order.
  • Test 63 pins the README claim that Grid::removeLocation leaves an entity's environment id unchanged.
  • Test 64 pins the README claim that Grid::addLocation stores a copy of its argument.
  • Test 65 pins that every move in a single-location grid returns false, including the random move.
  • Tests 66–67 pin removal of an entity added twice: one pointer survives when both adds targeted one location, and none survive when they targeted two locations.
  • Test 68 pins removal through a same-id twin, which leaves the stored entity's ids stale.
  • README.md documents the behaviours that were previously undocumented (getFirstEntity order, the console cell format, twin removal, and the one-pointer-per-location removal).

Test 66 characterizes behaviour that is believed to be a bug. It is filed as #70 and is deliberately not fixed here; the fix should update Test 66 and the matching README paragraph. This PR therefore does not close #70.

No tracking issue — gap found during triage.

Test plan

  • make compiles the suite locally
  • CI Library tests (make + CMake) job shows every Test N - ... through Test 68 followed by --- Success (local execution of the binary is denied in this environment, so the CI job is the anchor)
  • Every new test*() function is registered in main()

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

🤖 Generated with Claude Code


drafted by Claude on behalf of Daniel Stephenson

Pin printConsoleRepresentation's layout, getFirstEntity's location
order, and the removal behaviour of duplicated and same-id entities.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dmccoystephenson

Copy link
Copy Markdown
Member Author

Self-review rubric (run inline in this session, so it is not an independent review):

  • Scope: PASS — only src/tests.cpp and README.md are modified; git diff --name-only origin/master...HEAD lists nothing else, and no production code is touched.
  • Tests-new: PASS — no new public method was added. The test-side helper captureConsoleRepresentation is used by Tests 60–61.
  • Tests-fix: N/A — no bug fix is included. Test 66 characterizes the behaviour reported in Environment::removeEntity leaves an entity present when it was added twice to one location #70 rather than fixing it, so no stash-and-run applies.
  • Sibling structure: PASS — each new test follows the void test<Behavior>() shape with the Test N - ... / --- Success pair.
  • Sibling renames: N/A — nothing was renamed.
  • Docs: PASS — the ## Classes table is unchanged and still accurate. The four added README statements were each checked against src/ and a new test.
  • Issue resolution: N/A — no Closes reference is used; Environment::removeEntity leaves an entity present when it was added twice to one location #70 is deliberately left open.
  • Local test suite: PASS via CI — local execution of the binary is denied in this environment, so the Library tests (make + CMake) job was the anchor. Its log for run 41813267173 shows 68 Test N - ... --- Success lines, ending at Test 68. The Strata frontend and web build jobs also passed.
  • Header/definition parity: N/A — no header changed.
  • Include guards: N/A — no header was added.
  • Test registration: PASS — all nine new test*() functions are called from main(), and Tests 60–68 appear in the CI log.

Gaps noted:

  • src/tests.cpp Test 68 — the twin starts unplaced, so its ids are already at their defaults and the README's "resets only the twin's ids" is not observable from the twin. Only the half about the stored entity keeping stale ids is pinned. This is a coverage gap, not a wrong claim: the reset is visible in Grid::removeEntity and Location::removeEntity.
  • src/tests.cpp Test 66 — this test asserts behaviour believed to be a bug, so it is expected to change with the Environment::removeEntity leaves an entity present when it was added twice to one location #70 fix. Its name states what is being pinned so that the change is easy to find.
  • Environment::removeEntity leaves an entity present when it was added twice to one location #70 also reports that moveEntityToNewLocation leaves the entity in both locations. That claim was traced through the source but is not pinned by a test.
  • run_tests_asan.sh was not run. It is expected only for changes affecting allocation or object lifetime, and no production code changed.

Do-not-auto-merge paths: none matched. No workflow, security/, or Makefile path is modified, and no file has more than 50 lines deleted.


This comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).


drafted by Claude on behalf of Daniel Stephenson

@dmccoystephenson
dmccoystephenson merged commit 6095f30 into master Nov 4, 2026
3 checks passed
@dmccoystephenson
dmccoystephenson deleted the feature/pin-console-output-and-removal-quirks branch November 4, 2026 10:22
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.

Environment::removeEntity leaves an entity present when it was added twice to one location

1 participant