ci: pin Node 24, test every package, fix data-lit-todo id-shape assumption - #159
Merged
Conversation
…ption CI let a broken data-lit-todo conformance suite merge green (#158) for two reasons, both fixed here: 1. The downstream test step only ran six packages and *built* — never tested — the private sample apps. Broadened the filter to run `test` in every workspace package except the core (covered by the `data` job) and data-persistence (Playwright — kept on its `test:node` project). This now includes data-lit-todo and the other samples. 2. `Set.prototype.isSupersetOf` (used in the ecs readEntity path) is Node 22+ and several suites need it; CI ran Node 22 but nothing pinned it. Pinned Node >=24 (engines, .nvmrc, .node-version) and bumped both CI jobs to 24. The data-lit-todo conformance harness assumed ecs entity ids equalled the spec's domain ids (1..N) — true only under the old sign-based id scheme, broken by the persistence/sharing quadrant encoding (stride-4 ids). Made the harness id-shape-agnostic instead: fromState returns the seeded entities so the runner maps spec id -> entity, and the projection halves compare ignoring the todo id. No domain-id component added; the app is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the hand-rolled toEntity(1_000_000, 0) in the conformance harness with a first-class Entity.none: the last representable id (max local index in the last quadrant, numerically -1). The allocator issues local indices from 0 and a quadrant fills long before 2^30 entities, so it never collides with a live entity and locate/read resolve it to null. The bit layout stays owned by persistence-sharing.ts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Description
Post-merge cleanup for the persistence/sharing quadrant refactor (#158). That PR
merged green even though the
data-lit-todoconformance suite was red, becauseCI had two gaps. This PR fixes the cause of the failures and the CI blind
spot that let them through. No package version bump.
1. CI now tests every package (was: samples only built)
The downstream
teststep ran unit tests for only six packages and merelybuilt the private sample apps (
data-lit-todo,data-lit-tictactoe, …). Abroken sample unit test could therefore merge green. The step now runs
testin every workspace package except the core (already covered by the
datajob)and
data-persistence(whosetestpulls in Playwright — kept on itstest:nodeproject). pnpm skips packages that declare notestscript.2. Pin Node >= 24
Set.prototype.isSupersetOf(used on the ecsreadEntitypath) is Node 22+,and several suites depend on it, but nothing pinned the runtime. Pinned Node
>=24in rootengines, added/updated.nvmrcand.node-versionto24,and bumped both CI jobs from Node 22 to 24.
3. Fix
data-lit-todoconformance id-shape assumptionThe conformance harness assumed the ecs entity id equalled the spec's domain
id(1..Nin display order). That identity held only under the oldsign-based id scheme; the quadrant encoding gives document entities stride-4
ids (
0, 4, 8, …), so every id-addressed case broke.Made the harness id-shape-agnostic rather than re-coupling it to the new
encoding or adding a domain-id component:
fromStatereturns the seeded entities in display order; the runner mapsspec
id→ entity positionally and passes aresolveinto eachapply, soid-addressed transactions (delete / toggle / drag) target the right entity.
expectStateMatchesIgnoringIds, whichcanonicalises todo ids before comparison — behaviour is checked on the
visible fields, never on the id value. The pure-spec half stays id-strict
(the spec fully owns its domain ids).
The app model is unchanged; this touches test-only conformance code.
Verification
On Node 24, full CI-equivalent suite is green:
@adobe/data: typecheck + lint +test:ci— 2921 tests pass.test(incl. every sample app) pass;data-lit-todo79/79.
@adobe/data-persistencetest:node— 139 pass.Related PRs
🤖 Generated with Claude Code