test(ci): re-integrate coverage lost with dead FullGas relics (REINTEGRATE-ORPHAN-TESTS)#20
Merged
Merged
Conversation
Coverage audit found 62 features tested ONLY by the dead FullGas relics under tests/dcs/noPlayer/. 46 are genuine gaps (behaviour alive in src/, tested nowhere active), 10 are false orphans already covered elsewhere, 6 need FullGas (deep refactors / visual check — logged in dev/fullgas-report.md). This lot re-integrates the 46 gaps, busted-first (mockable -> tests/ci/ in CI; only real-engine behaviour stays as live-DCS tagged scenarios). It precedes and unblocks the planned CLEANUP-LEGACY-DCS-TESTS purge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lot) CTLDCrateManager loadCrate/unpackCrate/registerMMCrate + their OnCrate*/ OnMMCrateDetected events and guards — previously covered only by dead relics F-027/F-028/F-030/F-041. Complements crate_manager_spec.lua (entity level only). Pilot subset (no static respawn): validates the busted pattern in CI before the rest of the lot. F-029/031/032 (with addStaticObject mock) follow once green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reviewer's GuideReintegrates previously orphaned CTLD coverage by adding busted unit tests for crate lifecycle, deploy managers, menu gating, legacy wrappers, JTAC/recon behaviour plus a few live-DCS scenarios, and wires CI/DCS helpers and backlog docs to track the REINTEGRATE-ORPHAN-TESTS lot. Sequence diagram for updated dcs-bridge auth in integration runnersequenceDiagram
participant IntegrationRunner
participant DCSBridge
IntegrationRunner->>IntegrationRunner: http_post
IntegrationRunner->>DCSBridge: POST /api/exec Authorization Bearer api_key
DCSBridge-->>IntegrationRunner: response
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
…9/031/032) Adds the static-respawn cases to crate_lifecycle_spec: unloadCrate (LANDED + OnCrateUnloaded + re-register), dropCrate safe (method=drop) / destroy (drop_impact + unregister), and the not-loaded guard. Mocks coalition. addStaticObject + StaticObject.getByName as in crate_manager_spec's spawnCrate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
legacy_api_spec covers all 21 v1→v2 wrappers (Troops/Zones/Crates/Beacon/JTAC): each routes to the correct v2 manager method with args preserved and logs a DEPRECATED warning. An is_function guard asserts the routing target actually exists on the manager. Re-integrates dead relics F-094..F-098. Also loads src/legacy/legacy_api.lua in the busted helper (loader.lua) — it was missing from the module list, so the wrappers were unreachable from any spec. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…icket 06)
- F-110: JTAC drone config defaults (droneRadius/droneAltitude/lock) + per-
coalition JTAC drone descriptors. NB: the relic's JTAC_unitTypeNames key is
gone — the JTAC role is now declared via isJTAC=true in spawnableCrates;
re-integrated against the current model, not the dead key.
- F-112: deregisterJTAC is silent (no OnJTACDead), returns the laser code to the
pool, idempotent, unknown group is a no-op.
- F-011: recon enable/disableAutoRefresh toggle scan.autoRefresh and publish
OnReconAutoRefresh{Enabled,Disabled} with previous/new state.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- F-043/F-091: FARP Alpha (14 steps) and farpScene (6 steps) model structure via getModel(), asserting steps/registry keys/polar params against current src. - F-083/084/085: setLandMine mine counts — staggered formula T(N,R)=R*N-floor(R/2) gives 1 / 68 / 11 (verified by running the real src). - F-087: setLandMineAuto parametric count (39), single-mine branch, and guards. Loads src/scenes/* and i18n fr/es/ko in setup (same as minefield_spec/i18n_spec), since loader.lua does not pull those in. Complements minefield_spec (which covers structure + guards) with the actual counts and the FARP scenes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- F-021/022/023: CTLDCrateAssemblyManager tryUnpackOrRepair — complete assembly (OnAASystemDeployed + crates consumed), incomplete (no deploy), repair (OnAASystemRepaired). Template pulled from TEMPLATES, no hardcoded types. - F-012/013: CTLDFOBManager _registerDeployedFOB (OnFOBDeployed + _objectToFOB reverse lookup) and onDead/_destroyFOB (integrity threshold + OnFOBDestroyed + cleanup). - F-099: findPackableVehicles + packVehicle (OnVehiclePacked, cratesSpawned derived from descriptor.cratesRequired, deferred refresh via scheduleFunction). Assertions recomputed against current src (M1043 cratesRequired=3, deferred refresh, _repairFor contract), not the stale relic assumptions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
buildMenu F10 section gating by config flag / transport capability (F-048/049/ 050/052/053/054/055/056), clearBranch+repopulate (F-075), refreshMenuForGroup unknown-group failure shape (F-077), _loadUserConfig ingestion (F-088), troop Embark filter disabled/side/capacity (F-089), and player OnVehicleLoaded/ Unloaded -> loadedVehicles wiring (F-025). Deviations recomputed against current src (not the relic intent): FOBs is now a top-level "FOBs List" section, "Pack Vehicle" -> "Pack Equipt" with a combined enableFARPRepack/enablePackingVehicles gate, CTLDTroopManager:buildMenu gone -> buildMenuSection/refreshMenuSection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mock
Two CI-only failures the local harness could not surface:
- F-088 (_loadUserConfig): busted insulates each describe's environment, so a
bare `ctld_config_user = {...}` landed in the sandbox while _loadUserConfig
reads the real _G global — it saw nil and created no custom groups (3 templates
instead of 6). Write/read/restore via _G.ctld_config_user explicitly.
- F-056 (canCarryVehicles): buildFullMenu now stubs Unit.getByName for the build,
so the vehicle section's findLoadableVehicles gets a real unit object
(getTypeName/getCoalition/getPoint) instead of crashing on nil. Restored
immediately after the build to avoid polluting other specs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ket 07) Three tagged live-DCS scenarios for the relics that need the real engine (spawn, world events) and can't be busted-mocked: - scenario_beacon_lifecycle (F-006/007/092): dropBeacon spawn + OnBeaconDropped, removeClosestBeacon + OnBeaconRemoved, FOB beacon infinite battery. - scenario_recon_scan_payload (F-009): scan() OnReconScan payload + marks. - scenario_vehicle_dcs_native (F-018/019): loadVehicle/unloadVehicle dcs_native. Parity fixes vs the stale relics: in dcs_native the unit stays alive (relic expected unit==nil), unload -> WAITING, recon payload exposes totalTargetsDetected. Validated live against Test_CTLDNEXT_01: BCN 30/30, RCN 10/10, VEH 22/22. Also migrates the runner auth to Authorization: Bearer <token> (dcs-bridge LOT-018; /api/exec now requires the SUPERUSER role). Older X-API-Key is rejected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Several of the new DCS scenarios (e.g. beacon/vehicle/recon) duplicate common patterns for CTLD-ready guards, double-injection guards, anchor position resolution, and event-capture/cleanup; consider extracting these into a shared helper or template to reduce drift and keep the scenarios focused on scenario-specific assertions.
- Many of the new busted specs reach into manager internals (e.g.
_completeSystems,_fobs,_activeScans,_laserPool,_spawnedStatics); where feasible, prefer asserting through public methods or small helper accessors so that refactors of internal representations don’t require broad test rewrites.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Several of the new DCS scenarios (e.g. beacon/vehicle/recon) duplicate common patterns for CTLD-ready guards, double-injection guards, anchor position resolution, and event-capture/cleanup; consider extracting these into a shared helper or template to reduce drift and keep the scenarios focused on scenario-specific assertions.
- Many of the new busted specs reach into manager internals (e.g. `_completeSystems`, `_fobs`, `_activeScans`, `_laserPool`, `_spawnedStatics`); where feasible, prefer asserting through public methods or small helper accessors so that refactors of internal representations don’t require broad test rewrites.
## Individual Comments
### Comment 1
<location path="tools/integration-runner/run_scenarios.py" line_range="263-264" />
<code_context>
+ # endpoint (/api/exec requires the SUPERUSER role). The api_key from the config
+ # is used as that token. (Older builds used an X-API-Key header.)
+ headers={"Content-Type": "application/json",
+ "Authorization": "Bearer " + api_key},
method="POST",
)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider more robust construction of the Authorization header.
String-concatenating the scheme assumes `api_key` has no extra whitespace or existing scheme. Prefer constructing the header as `f"Bearer {api_key.strip()}"` (or validating/normalizing `api_key` at load time) to avoid subtle auth failures from non-normalized config values.
```suggestion
headers={
"Content-Type": "application/json",
"Authorization": f"Bearer {api_key.strip()}",
},
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+263
to
+264
| headers={"Content-Type": "application/json", | ||
| "Authorization": "Bearer " + api_key}, |
There was a problem hiding this comment.
suggestion (bug_risk): Consider more robust construction of the Authorization header.
String-concatenating the scheme assumes api_key has no extra whitespace or existing scheme. Prefer constructing the header as f"Bearer {api_key.strip()}" (or validating/normalizing api_key at load time) to avoid subtle auth failures from non-normalized config values.
Suggested change
| headers={"Content-Type": "application/json", | |
| "Authorization": "Bearer " + api_key}, | |
| headers={ | |
| "Content-Type": "application/json", | |
| "Authorization": f"Bearer {api_key.strip()}", | |
| }, |
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
1 participant
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.
Context
A coverage audit found 62 features tested only by the ~194 dead FullGas relics under
tests/dcs/noPlayer/(no@tierheader,dofileof an absentDCS-CTLD_FG/recette/setup.lua,missing
ctld_testframework — they execute nothing). Breakdown:src/, tested nowhere active → re-integrated by this lot.dev/fullgas-report.md).This lot rebuilds the 46, busted-first (mockable →
tests/ci/in CI; only real-enginebehaviour stays as live-DCS tagged scenarios). It precedes and unblocks the planned
CLEANUP-LEGACY-DCS-TESTSpurge.See
.backlog/REINTEGRATE-ORPHAN-TESTS/PRD.mdfor the full audit + the 7-ticket plan.This PR so far (draft)
crate_lifecycle_spec.lua:CTLDCrateManagerloadCrate/unpackCrate/registerMMCrate + events/guards (F-027/028/030/041). Pilot subset (no static respawn) to
validate the busted pattern in CI; F-029/031/032 + tickets 02–06 follow once green.
Draft until the busted lots (01–06) are complete; ticket 07 (live-DCS) is validated separately
against a running mission.
🤖 Generated with Claude Code
Summary by Sourcery
Reintegrate test coverage for previously untested CTLD features and DCS behaviours, and wire them into CI and integration tooling.
New Features:
Enhancements:
Documentation:
Tests: