v0.6.59 — backend test coverage, dead-code cleanup and defect fixes

[roncodes]

v0.6.59

Release branch consolidating the Fleet-Ops backend test-coverage campaign, the dead-code cleanup it uncovered, and the production defects found along the way.

⚠️ This must not merge to main until fleetbase/core-api 1.6.55 is released. One test on this branch asserts a contract that only holds after that fix — see "Known failing test" below.

What's in this release

Backend test coverage: 79.53% → 99.74%

server/src line coverage went from 79.53% to 99.74%. Tests are organised under server/tests/Unit/... and server/tests/Feature/Http/{Api,Internal}/... rather than adding to the flat root-level sprawl.

Milestone	Line coverage	Uncovered statements
Campaign start	79.53%	~7,000
#277 + #281 consolidated	99.57%	145
Relation callbacks and push channels	99.62%	131
Observer guards, metric queries, geojson fallbacks	99.67%	114
Shift, simulation, analytics and registry seams	99.69%	104
Relation fallbacks, import defaults and skip guards	99.74%	89

The coverage gate itself is wired into the Composer workflow — composer coverage:baseline writes a Clover report and composer coverage:check enforces --fail-under=100. The gate is intentionally still red; see "Remaining work".

Fifteen production bugs found and fixed

Examining every uncovered line turned up real defects, not just missing tests. Highlights:

• Place::insertFromMixed() crashed on any plain address string. It called insertFromGeocodingLookup(), which existed nowhere on Place or any ancestor — every such call raised BadMethodCallException. Defined as the insert-side twin of createFromGeocodingLookup, mirroring insertFromGoogleAddress.
• Invalid coordinates were silently reverse-geocoded at Null Island. GeocoderController::reverse() validated after converting with getPointFromCoordinates(), which is typed : Point and falls back to Point(0, 0) — so the "Invalid coordinates provided." guard never fired. An existing test had codified the buggy behaviour.
• Place::insertFromCoordinates() never detected empty reverse-geocoding results — !$results->count() === 0 compares a bool to an int, so places were silently inserted at 0,0 instead of returning false.
• Lalamove::getQuotationForMarket() passed the market into the bool $sandbox slot, so market-scoped quotations silently ran against the sandbox host with the market dropped.
• OrderConfig::default() declared a non-nullable self return while returning first(), fataling for companies without a stored transport config.
• ServiceRate called Collection::sortByDesc() with no argument, throwing whenever a parcel outsized every fee tier.

Unreachable code removed or repaired

A large share of what looked like "untested" code turned out to be unreachable — branches shadowed by a broader arm above them, guards on values a type declaration forbids, fallbacks after an unconditional assignment. Each was handled deliberately:

• Reordered where the guard's intent was real, so it now fires with behaviour unchanged (the spatial casts).
• Deleted only where the shadowing arm was genuinely equivalent.
• Annotated @codeCoverageIgnore with a reviewable reason where a type or an earlier validate() makes the state impossible.

One case is worth calling out: Casts/MultiPolygon's shadowed arm returned a bare geometry while the arm above it wraps in SpatialExpression. Reordering it verbatim would have silently flipped MultiPolygon writes from wrapped to bare — on the zone/service-area/location write path. It was reordered and matched, and behaviour preservation is evidenced by the spatial suites passing with their original assertions.

Found while covering — not fixed here

Utils::getPointFromMixed() has two fallback arms (Support/Utils.php:275 and :279) that recurse with a bare coordinate pair pulled out of a GeoJSON envelope. The array reader at :296–:297 resolves positionally, taking index 0 as latitude and 1 as longitude — the reverse of GeoJSON's [lng, lat]. A pair that reaches either fallback therefore comes back transposed.

server/tests/PointResolutionTest.php asserts the behaviour as it stands, with a comment saying so. Correcting it touches every location write path, so it is deliberately left for its own change rather than folded into a coverage commit.

Known failing test

server/tests/Feature/Http/Internal/OrderControllerUpstreamNotFoundTest.php fails on this branch, deliberately.

Internal\v1\OrderController::nextActivity() wraps Order::findByIdOrFail() in a catch (ModelNotFoundException) that never fires today: core-api's findByIdOrFail() calls a getModelNotFoundException() method that does not exist on Eloquent's builder, so a missing order raises BadMethodCallException, escapes the catch, and surfaces as a 500 instead of a 404. core-api#231 fixes it on dev-v1.6.55.

The test asserts the post-fix contract on purpose — asserting today's BadMethodCallException would codify the defect. It currently fails with exactly Call to undefined method Builder::getModelNotFoundException() and turns green when the release lands.

To keep this branch measurable while that is outstanding, scripts/coverage-file-runner.php gained an opt-in FLEETOPS_COVERAGE_CONTINUE_ON_FAILURE=1: a failing file is recorded rather than aborting the run, the Clover report is still written, and the process still exits non-zero. Without it, the runner exit()s inside its per-file loop — every later file is skipped, no report is written, and the stale clover.xml is left behind.

Remaining work before this ships

Roughly 89 statements are still uncovered, in three groups:

• Coverable — the bulk. Guard branches, controller arms and import fallbacks reachable by shaping the input, following the same seam and sweep patterns used so far.
• Provably dead (~19) — shadowed branches, guards on values a type declaration forbids, and method_exists checks on methods that are declared. These get the delete-or-annotate treatment already applied in Remove unreachable backend code and fix two defects it was hiding #281. Two were reclassified during execution: OrderDispatched:151-152 and OrderPing:158-159 sit behind a method_exists($resource, 'toWebhookPayload') check where $resource is an unconditional new OrderResource(...) and Order::toWebhookPayload() is declared — so the elseif can never run.
• Environment-blocked (~4) — the extension_loaded('geos') branch, the "core-api absent" throws, and ProofController::createSignatureFile, whose File creation resolves a disk, a url and app()->environment() and so needs a real Application rather than the container the harness builds.

Then wire composer coverage:check into .github/workflows/server.yml, and pull core-api 1.6.55 to confirm the gated test turns green.

Scope note on the test suite

Worth being explicit, since a coverage percentage invites over-reading: this suite runs on in-memory SQLite with eval'd function shims, container stand-ins and hand-registered spatial UDFs. It is good at proving branch logic and contracts, and poor at proving real-MySQL behaviour — the spatial write paths especially, where a fixture's prepareBindings override compensates for a real cast divergence. A green run means "the branches behave as described", not "the feature works against a real database". Manual verification against MySQL is still warranted for zones, service areas, and location save/update.