From 7935f2b18edb4ac3041f71036b2e38e61af36923 Mon Sep 17 00:00:00 2001 From: Sebastian Dine Date: Mon, 11 May 2026 08:42:07 +0200 Subject: [PATCH] fix: Fix/ci cd issues (#12) --- AGENTS.md | 7 ++++++- core/src/services/CardFilter.cpp | 1 + core/src/services/CardSorter.cpp | 1 + tests/AGENTS.md | 3 ++- tests/CMakeLists.txt | 2 ++ tests/domain_json_tests.cpp | 5 +++-- 6 files changed, 15 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7c503cd..6432964 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -52,7 +52,7 @@ Run from the **workspace root**. - Run the app: `./build/bin/ccm3` (`.\build\bin\ccm3.exe` on Windows) - Run tests (CCM_BUILD_TESTS defaults to ON): - `ctest --test-dir build --output-on-failure` — current baseline: **215 tests, all green**. + `ctest --test-dir build --output-on-failure` — current baseline: **226 tests, all green**. - Build tests only: `cmake --build build --target ccm_core_tests` - Local coverage env setup (one-time, Windows/MSYS2): @@ -112,6 +112,11 @@ Run from the **workspace root**. - After changing the per-game seams (`IGameModule`, `IGameView`, the `BaseCard*Panel` template hooks) you **must** update `docs/adding-a-new-game.md` so the canonical "add a new game" walkthrough stays in sync with the code. - After changing `formatTextForFs` or `parseIndexFromFilename` you **must** update `tests/fs_names_tests.cpp` — these functions exist to stay byte-compatible with the original Rust `util/fs.rs`. +## Agent collaboration (Cursor / AI) + +- **Never** `git commit` or `git push` unless the user **explicitly** asked you to commit and/or push (e.g. “commit this”, “push to origin”). Preparing diffs and suggesting commands is fine; performing those Git writes without explicit instruction is not. +- **Never** check out another branch **to change it** unless the user **explicitly** asked you to work on that branch. Temporarily checking out another branch **read-only** (inspect history, compare files, run `git show`) is fine without asking; switch back to the working branch before making edits unless instructed otherwise. + ## Anti-patterns - Don't include `wx/...` headers from `core/` (breaks layering and tests will refuse to build). diff --git a/core/src/services/CardFilter.cpp b/core/src/services/CardFilter.cpp index 37a9970..24d02d7 100644 --- a/core/src/services/CardFilter.cpp +++ b/core/src/services/CardFilter.cpp @@ -2,6 +2,7 @@ #include "ccm/domain/Enums.hpp" #include "ccm/util/AsciiUtils.hpp" +#include "ccm/util/YuGiOhPrintingSlot.hpp" #include #include diff --git a/core/src/services/CardSorter.cpp b/core/src/services/CardSorter.cpp index 594f7fe..a6584e5 100644 --- a/core/src/services/CardSorter.cpp +++ b/core/src/services/CardSorter.cpp @@ -2,6 +2,7 @@ #include "ccm/domain/Enums.hpp" #include "ccm/util/AsciiUtils.hpp" +#include "ccm/util/YuGiOhPrintingSlot.hpp" #include #include diff --git a/tests/AGENTS.md b/tests/AGENTS.md index 21719d4..45c2d32 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -17,11 +17,12 @@ - `magic_set_source_tests.cpp` — `MagicSetSource::parseResponse` (Scryfall mapping). Drives `fetchAll` via `FixedHttpClient` fake. - `magic_card_preview_source_tests.cpp` — `MagicCardPreviewSource::buildSearchUrl` URL-encoding rules + `parseResponse` (`data[0].image_uris.normal`). Drives `fetchImageUrl` via `FixedHttpClient`. - `card_preview_service_tests.cpp` — `CardPreviewService` registry/orchestration through `registerModule(IGameModule&)` with an inline `FakeGameModule` returning a `FakeSource : ICardPreviewSource` (which carries a `PreviewLookupError::Kind` knob so tests can drive both transient and not-found paths) and a `FixedHttpClient`. Both fakes count `calls` so cache-hit assertions are precise. Pin-downs include: "module returning nullptr is silently skipped", the per-game `detectFirstPrint` / `detectPrintVariants` opt-in guards, and the LRU bytes cache (repeat `fetchPreviewBytes` for the same `(game, name, setId, setNo)` returns the cached payload without touching the source or HTTP; different cards get separate cache slots; transient errors are **not** cached so a flaky connection recovers; `fetchImageBytesByUrl` is keyed by URL and serves the per-game card-back fallback from the same LRU). Production `fetchAndCache` rejects empty HTTP bodies (not exercised by these fakes unless a test sets an empty `body` deliberately). The negative-cache behavior is also pinned down: a `NotFound` source error writes through to the persistent cache *and* short-circuits the next lookup (source not re-invoked); editing a lookup-relevant field invalidates the negative entry automatically; warm-restart (a fresh service over the same cache fake) honors a previously stored negative entry; and a later positive result for the same key replaces the negative entry. The persistent-tier wiring uses an inline `InMemoryByteCache : IPreviewByteCache` fake whose `Entry { negative, payload }` carries the kind explicitly. -- `local_preview_byte_cache_tests.cpp` — `LocalPreviewByteCache` adapter against `StdFileSystem` (the only test in the suite that touches real disk; each case scopes itself to a unique `temp_directory_path()/ccm_preview_cache_test_*` directory and cleans up via an RAII `TempDir`). Pin-downs: store/load round-trips bytes verbatim; missing key is a clean miss; empty payload is silently skipped; sidecar mismatch (faked hash collision) is treated as a miss so we never serve the wrong card's bytes (or wrong card's negative verdict); the cache survives an adapter restart over the same directory; total-size eviction drops the oldest `.bin` by mtime when a `store` would exceed the cap; a `load` touches the entry's mtime so frequently-viewed cards survive eviction. Negative-entry coverage: `storeNegative` round-trips as `NegativeHit` (not a miss, not a payload, and not counted against the byte cap); negatives survive an adapter restart; a later positive `store` overwrites a previous negative and a later `storeNegative` overwrites a previous positive (releasing its bytes from the cap); and the sidecar collision check applies to negative entries too. +- `local_preview_byte_cache_tests.cpp` — `LocalPreviewByteCache` adapter against `StdFileSystem` (real disk under a unique `temp_directory_path()/ccm_preview_cache_test_*` per case, RAII `TempDir` cleanup; see also `std_file_system_tests.cpp`). Pin-downs: store/load round-trips bytes verbatim; missing key is a clean miss; empty payload is silently skipped; sidecar mismatch (faked hash collision) is treated as a miss so we never serve the wrong card's bytes (or wrong card's negative verdict); the cache survives an adapter restart over the same directory; total-size eviction drops the oldest `.bin` by mtime when a `store` would exceed the cap; a `load` touches the entry's mtime so frequently-viewed cards survive eviction. Negative-entry coverage: `storeNegative` round-trips as `NegativeHit` (not a miss, not a payload, and not counted against the byte cap); negatives survive an adapter restart; a later positive `store` overwrites a previous negative and a later `storeNegative` overwrites a previous positive (releasing its bytes from the cap); and the sidecar collision check applies to negative entries too. - `std_file_system_tests.cpp` — `StdFileSystem` directly (`exists`, `isDirectory`, `ensureDirectory`, `readText`, `writeText`, `copyFile`, `remove`, `listDirectory`) under a unique `temp_directory_path()/ccm_std_fs_test_*` directory per case; scope matches the real-disk exception documented for preview-cache tests. - `pokemon_set_source_tests.cpp` — `PokemonSetSource::parseResponse` (api.pokemontcg.io/v2/sets shape — `data[].id`, `name`, `releaseDate` already in `YYYY/MM/DD`) + sort-by-release-date stability. Drives `fetchAll` via `FixedHttpClient` and asserts the public endpoint URL. - `pokemon_card_preview_source_tests.cpp` — `PokemonCardPreviewSource::buildSearchUrl` (percent-encoded `name:` / `set.id:` / `number:` triple, with collector-number `4/102` -> `4` normalization) + `parseResponse` (`data[0].images.large` with `images.small` fallback). Drives `fetchImageUrl` via `FixedHttpClient`. - `yugioh_set_source_tests.cpp` — `YuGiOhSetSource::parseResponse` for YGOPRODeck `cardsets.php` (`set_code`, `set_name`, `tcg_date`) including `YYYY-MM-DD` -> `YYYY/MM/DD` rewrite and chronological sort checks. +- `game_module_tests.cpp` — smoke tests that each concrete `IGameModule` (Magic / Pokemon / Yu-Gi-Oh) reports stable `id()`, `dirName()`, `displayName()`, and a non-null `cardPreviewSource()` when constructed with a noop `IHttpClient`. - `yugioh_card_preview_source_tests.cpp` — `YuGiOhCardPreviewSource` Yugipedia + YGOPRODeck unit coverage. Helper-level tests pin down `normalizeName` (whitespace + Yugipedia-policy punctuation stripping), `ygoRarityShortCode` + `rarityCodeFor` (CCM3 dialog rarity names → canonical short codes used by both the YGO overview table and Yugipedia filename generation; unknown rarity falls through), `extractSetCode` (`LOB-005` / `LOB-DE005` → `LOB`), `buildCandidateFilenames` (printed-edition first, EN/NA/EU/AU + png/jpg, rarity-less fallback round, empty list when slug or set code is missing), `buildYugipediaQueryUrl` (single `titles=File:A|File:B` batch, percent-encoded), and `parseYugipediaResponse` (returns the URL of the highest-priority filename that resolved, errors when every candidate is `missing`). End-to-end `fetchImageUrl` cases use a `RoutingHttpClient` to verify Yugipedia is queried first and the per-printing scan is returned when found, that empty/error Yugipedia responses fall through to the YGOPRODeck `card_images[0]` fallback, that the YGOPRODeck error is propagated when both upstreams fail, and that an empty `setNo` skips Yugipedia entirely. `parseFirstPrint` preferred-`set_name` lookup is also covered for the auto-detect path. `parsePrintVariants` includes synthetic scenarios aligned with the `yugioh_same_card_set_variant_tests` fixture (dual-rarity vs multi-code within one display set, duplicate suppression, and no merge across unrelated `set_name` rows when the picker label matches nothing). - `card_sorter_tests.cpp` — `sortMagicCards` / `sortPokemonCards` per-column behavior. Pin-down tests for `byField`-equivalent semantics: case-insensitive strings, chronological set sort via `set.releaseDate`, numeric `amount`, `false < true` boolean order, stable composition (sort by name then by set keeps inner-name order). Update this file whenever you add a new column / sort key. - `card_filter_tests.cpp` — `matchesMagicFilter` / `matchesPokemonFilter` / `matchesYuGiOhFilter` row-matcher behavior. Pin-down tests for `applyFilter`-equivalent semantics: case-insensitive substring match across `tableFields` valueKeys (name, set.name, language, condition, amount-as-string, note; Pokemon adds `setNo`; Yu-Gi-Oh adds `setNo` + `rarity`), boolean flag columns intentionally excluded, empty filter matches everything. Update this file whenever you add a new searchable column. diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 91911dc..ac019eb 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -18,10 +18,12 @@ add_executable(ccm_core_tests magic_card_preview_source_tests.cpp card_preview_service_tests.cpp local_preview_byte_cache_tests.cpp + std_file_system_tests.cpp pokemon_set_source_tests.cpp pokemon_card_preview_source_tests.cpp yugioh_set_source_tests.cpp yugioh_card_preview_source_tests.cpp + game_module_tests.cpp card_sorter_tests.cpp card_filter_tests.cpp http_get_mapping_tests.cpp diff --git a/tests/domain_json_tests.cpp b/tests/domain_json_tests.cpp index 1452a9a..115c85a 100644 --- a/tests/domain_json_tests.cpp +++ b/tests/domain_json_tests.cpp @@ -209,7 +209,7 @@ TEST_SUITE("YuGiOhCard JSON") { CHECK(back == c); } - TEST_CASE("missing rarityCode in legacy rows is accepted and mapped to empty") { + TEST_CASE("legacy JSON without rarityCode key parses (domain uses rarity only)") { const nlohmann::json j = { {"id", 1}, {"amount", 1}, @@ -232,7 +232,8 @@ TEST_SUITE("YuGiOhCard JSON") { const auto card = j.get(); CHECK(card.rarity == "Ultra Rare"); - CHECK(card.rarityCode.empty()); + CHECK(card.setNo == "SDY-006"); + CHECK(card.set.id == "SDY"); } }