funman300/Ferrous-Solitaire
1
1
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases 48 Wiki Activity

Remove TestPileState #85

New Issue
Closed
opened 2026-06-11 03:14:13 +00:00 by Quaternions · 1 comment
Quaternions commented 2026-06-11 03:14:13 +00:00
Copy Link
Copy Source

Test infrastructure should never appear in final types, yet for some reason this appears in solitaire_core/src/klondike_adapter.rs GameState in the test_pile_state field.

Test infrastructure should never appear in final types, yet for some reason this appears in solitaire_core/src/klondike_adapter.rs GameState in the test_pile_state field.
funman300 commented 2026-06-12 02:48:58 +00:00
Owner
Copy Link
Copy Source

Investigated — TestPileState does NOT appear in the final/production types. It's already correctly gated out.

  • test_pile_state is #[cfg(feature = "test-support")], and test-support is a dev-only feature: solitaire_engine/solitaire_data enable it in [dev-dependencies], not [dependencies].
  • The workspace uses resolver = "2" (edition 2024), which isolates dev-dependency features from normal builds.
  • cargo tree -p solitaire_app -i solitaire_core -e features shows the production app pulls solitaire_core with only the default feature — no test-support.
  • cargo build -p solitaire_app compiles cleanly with no test-support (verified), so the field + every set_test_* method are compiled out of the shipped binary. The lone production caller, has_test_pile_overrides(), has a #[cfg(not(feature="test-support"))] fallback returning false.

So the field appears only in source (under the cfg gate) and during test builds — never in a final type.

Fully deleting it from source isn't viable: the ~91 test sites inject arbitrary hand-crafted boards (e.g. a lone 5♣ in Tableau1) that are unreachable by legal play, and the override is checked inside core's own pile-read methods (so the field must live on GameState). Relocating that mechanism out of core is a large refactor for a field already absent from production.

Recommend: close as 'already gated out of production'. Optional modest cleanup: move the set_test_* impl block + TestPileState into a dedicated #[cfg(feature="test-support")] module to declutter game_state.rs (field stays on GameState; ~150 lines of setters move out).

**Investigated — TestPileState does NOT appear in the final/production types.** It's already correctly gated out. - `test_pile_state` is `#[cfg(feature = "test-support")]`, and `test-support` is a **dev-only** feature: `solitaire_engine`/`solitaire_data` enable it in `[dev-dependencies]`, not `[dependencies]`. - The workspace uses `resolver = "2"` (edition 2024), which isolates dev-dependency features from normal builds. - `cargo tree -p solitaire_app -i solitaire_core -e features` shows the production app pulls `solitaire_core` with only the `default` feature — no `test-support`. - `cargo build -p solitaire_app` compiles cleanly with no test-support (verified), so the field + every `set_test_*` method are compiled out of the shipped binary. The lone production caller, `has_test_pile_overrides()`, has a `#[cfg(not(feature="test-support"))]` fallback returning `false`. So the field appears only in **source** (under the cfg gate) and during **test** builds — never in a final type. Fully deleting it from source isn't viable: the ~91 test sites inject **arbitrary hand-crafted boards** (e.g. a lone 5♣ in Tableau1) that are unreachable by legal play, and the override is checked inside core's own pile-read methods (so the field must live on `GameState`). Relocating that mechanism out of core is a large refactor for a field already absent from production. Recommend: close as 'already gated out of production'. Optional modest cleanup: move the `set_test_*` impl block + `TestPileState` into a dedicated `#[cfg(feature="test-support")]` module to declutter game_state.rs (field stays on GameState; ~150 lines of setters move out).
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
android
bug
correctness
enhancement
performance
security
server
test
ui
ux
wasm
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
funman300 (Alex)
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: funman300/Ferrous-Solitaire#85
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?