test(model): assert updateAll multi-include by row identity, not float equality

[bpamiri]

Closes #3294.

The issue's premise is wrong, and that is the finding

#3294 reported updateAll(include=) with multiple associations silently updating 0 rows on MySQL — "a silent bulk-update no-op on a mainstream database", present in shipped v4.0.4 and v4.0.5.

It does not do that. updateAll returns 1 and the row is correctly updated, on every engine. The spec's verification step was unsound.

averagerating is a 4-byte float column. 3.3 has no exact binary representation, so it widens to 3.299999952316 — and MySQL evaluates averagerating = '3.3' as FALSE against the stored float. Proven directly against the container:

SELECT CAST(3.3 AS float) = 3.3;   -- 0
SELECT CAST(5.0 AS float) = 5.0;   -- 1

That is exactly why crudSpec's averagerating = '5.0' assertions (lines 1857-1860) pass while this one did not: 5.0 is exactly representable and 3.3 is not. Same pattern, different literal.

The include count is irrelevant

The issue inferred "the single-include case passes on MySQL, so it's specifically the second join that breaks row matching." Measured on lucee7 + mysql, each case in its own rolled-back transaction:

case	updateAll returned	rows found by = '3.3'
no include	1	0
one include	1	0
two includes	1	0
two includes, value 5.0	1	1 (= '5.0')

The no-include case fails the same assertion, so the joins were never implicated. The same four cases on SQLite all report found=1 — only the assertion varied by engine, never the update.

I also captured the generated MySQL SQL to rule out the UPDATE ... JOIN path:

UPDATE `c_o_r_e_posts`
  INNER JOIN `c_o_r_e_authors` ON `c_o_r_e_posts`.`authorid` = `c_o_r_e_authors`.`id`
  LEFT OUTER JOIN `c_o_r_e_comments` ON `c_o_r_e_posts`.`id` = `c_o_r_e_comments`.`postid`
SET averagerating = ?
WHERE ( `c_o_r_e_comments`.`postid` = ? AND `c_o_r_e_authors`.`id` > ? )
  AND ( `c_o_r_e_posts`.`deletedat` IS NULL )

Run by hand it reports ROW_COUNT() = 1 and post 1 reads back 3.3. The SQL is correct.

The change

The spec now verifies observable database state instead of a float equality:

• a positive rows-affected count — rules out a genuine no-op without pinning a driver-specific number (MySQL's UPDATE ... JOIN matches 3 join rows but 1 base row)
• the exact id list of rows carrying the new value

This is strictly stronger than what it replaced. The old assertion reported 0 on MySQL whether the join was right or wrong. Red-checked the new one by widening the WHERE to over-match:

Expected [1] but received [1,2,3,4,5]

No framework change and no changelog fragment — there was no user-facing defect to announce.

Verification

lucee7, full core suite, same container:

leg	result
mysql, develop	4720 pass / 8 fail / 4 error
mysql, this branch	4721 pass / 7 fail / 4 error
sqlite, this branch	4713 pass / 7 fail / 4 error

Failure-set diff on mysql: exactly one spec moves develop → branch (this one), zero new failures.

The 11 remaining failures are an identical pre-existing cluster on both branches and both databases (app.controllers.Controller missing its mixed-in helpers — filters(), linkTo(), startFormTag(), …). They are local to my container; CI's lucee7+mysql leg does not show them, so I have not chased them.

Two things I noticed but did not touch

1. vendor/wheels/model/update.cfc:92 concatenates join strings with no separator. local.list &= local.associations[local.i].join produces ...`id`LEFT OUTER JOIN... — malformed SQL that only parses because MySQL's backticks self-delimit the preceding identifier. Benign today since $quoteIdentifier always quotes, but it is a one-space fix away from being robust rather than lucky. Left alone as out of scope; happy to do it as a follow-up.
2. This is the first of the ten root causes in the refreshed matrix leg-debt inventory to be resolved. It was the only one with a tracking issue. Worth noting for compat-matrix: engine-job failures are invisible (continue-on-error) — harden after burning down pre-existing leg debt #3302 that its first burn-down item turned out to be a bad test rather than product debt — the other nine may not follow the same pattern.

[wheels-bot]

Wheels Bot — Reviewer

TL;DR — This PR rewrites one assertion in UpdateAllIncludeJoinSpec.cfc so that updateAll(include=) with multiple associations is verified by observable row identity and a positive rows-affected count instead of a float-equality check against the literal 3.3. The root-cause analysis (a 4-byte float column cannot satisfy averagerating = 3.3 on MySQL because 3.3 has no exact binary representation) is correct, and the replacement assertion is strictly stronger. Verdict: approve.

Correctness

The new assertions hold against the actual fixtures. Initial post ratings in vendor/wheels/tests/populate.cfm:549-577 are NULL, 3.6, 3.2, 3.6, 3.6 — none in the (3.29, 3.31) window. The updateAll where clause (c_o_r_e_comments.postid = 1) matches only post 1, which is set to 3.3, so findAll(where = "averagerating > 3.29 AND averagerating < 3.31", order = "id") resolves to exactly ValueList(loc.q.id) == "1". Values are captured into CFML variables before transaction action="rollback", so the post-transaction assertions read valid data. The looser expect(loc.state.updated).toBeGT(0) is justified inline (MySQL UPDATE ... JOIN reports a driver-specific affected-row count), with exactness carried by the row-identity check — a reasonable split, not a weakness.

Cross-engine

No new hazards. ValueList(query.col) is standard across Lucee/Adobe/BoxLang, toBeGT is already used at line 19 of this same spec, and the loc.* struct usage mirrors the pre-existing pattern in this file. The range window is wide enough to absorb the 3.299999952316 float widening on MySQL while staying clear of the 3.2 and 3.6 neighbors on exact-storage engines.

Tests

This is a test change — it greens a genuinely red lucee7 + mysql leg (per the pass/fail diff in the PR body) by replacing an unsound verification step. No framework code changed, so no changelog fragment is warranted; the author called that out explicitly and correctly.

Commits

test(model): assert updateAll multi-include by row identity, not float equality — conforms to commitlint.config.js (valid type test, scope model, subject <= 100 chars, not ALL-CAPS), and the subject describes the intent rather than the mechanics.

---

Non-blocking note (out of scope, not a finding against this PR): the PR body flags vendor/wheels/model/update.cfc:92 concatenating join strings with no separator, benign today only because $quoteIdentifier self-delimits with backticks. Worth a follow-up as the author suggests.

[github-actions]

Wheels Test Results

     31 files    9 744 suites   20m 50s ⏱️
130 837 tests 130 367 ✅ 397 💤 38 ❌ 35 🔥
132 769 runs  132 299 ✅ 397 💤 38 ❌ 35 🔥

For more details on these failures and errors, see this check.

Results for commit b4ada16.