Skip to content

Add tests pinning entity identity and duplicate-add behaviour - #69

Merged
dmccoystephenson merged 1 commit into
masterfrom
feature/pin-entity-identity
Oct 31, 2026
Merged

dmccoystephenson merged 1 commit into
masterfrom
feature/pin-entity-identity

Conversation

@dmccoystephenson

Copy link
Copy Markdown
Member

Summary

Stage B (unit-test expansion) cycle. The README's "Entity Identity" section describes three behaviours that no test pinned; three characterization tests are added to src/tests.cpp and registered in main():

  • Test 57 — testPlacingAndRemovingEntityResetsAllIds: a new entity starts with environment id -1, grid id -1 and location id "N/S"; placing it through an Environment sets all three, and Environment::removeEntity resets all three. (The existing tests checked only the environment id on removal and never checked the "N/S" default of a freshly constructed entity.)
  • Test 58 — testEntitiesSharingAnIdAreMatchedAsTheSameEntity: a distinct Entity object sharing an id is reported present by Location/Environment::isEntityPresent, getEntity returns the originally placed object, and Location::removeEntity called with the twin removes the original's pointer.
  • Test 59 — testAddingEntityTwiceStoresItTwice: adding the same entity twice stores two pointers, getNumEntities counts each, and the location id names only the most recent location.

No production code is changed. No open issues existed at triage, so no issue is closed and none was deferred.

Observation, not changed here: when an entity is removed through a same-id twin, Location::removeEntity resets the twin's location id, while the stored original keeps its stale location id. Test 58 deliberately does not assert either object's location id after that removal, so the quirk is not pinned.

Test plan

  • make compiles cleanly locally
  • CI "Library tests" job green (local execution of tests_executable is not permitted in this environment, so CI is the anchor)

No tracking issue — gap found during triage.

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

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

Copy link
Copy Markdown
Member Author

Self-review rubric:

  • Scope: PASS — git diff --stat origin/master shows only src/tests.cpp (+55), all of it the three new tests and their registrations.
  • Tests-new: PASS — no new public methods; three characterization tests added for README "Entity Identity" claims that had no test.
  • Tests-fix: N/A — no production code changed, so there is no fix to stash and revert.
  • Sibling structure: PASS — each test follows the existing void test<Behavior>() + "Test N - ..." / " --- Success" pattern, numbered 57–59 after Test 56.
  • Sibling renames: N/A — nothing renamed.
  • Docs: PASS — README's class table and "Entity Identity" section are unchanged and match what the tests now assert.
  • Issue resolution: N/A — no Closes #N; the gap was found during triage.
  • Local test suite: PASS (via CI) — local execution of tests_executable is denied in this environment; CI run 41176439432, job "Library tests (make + CMake)", printed Test 57 ... --- Success, Test 58 ... --- Success, Test 59 ... --- Success through the last registered test, with conclusion success. Browser build (run 41176439442) also succeeded.
  • Header/definition parity: N/A — no header changes.
  • Include guards: N/A — no new headers.
  • Test registration: PASS — all three functions are called at the end of main().

Judgment note: src/tests.cpp (Test 58) — the test intentionally leaves the location ids unasserted after removal through a same-id twin. Location::removeEntity resets the argument's (twin's) location id, so the stored original keeps a stale id. Pinning that would lock in arguably surprising behaviour, so it was left unpinned and is flagged here instead.

Test-only, anchor green, no do-not-auto-merge paths touched; ready to merge.

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


drafted by Claude on behalf of Daniel Stephenson

@dmccoystephenson
dmccoystephenson merged commit 0c29efc into master Oct 31, 2026
3 checks passed
@dmccoystephenson
dmccoystephenson deleted the feature/pin-entity-identity branch October 31, 2026 06:27
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.

1 participant