Skip to content

fix: reject ordered member sequences longer than the target - #1857

Open
emme1t wants to merge 1 commit into
chaijs:mainfrom
emme1t:fix/ordered-members-undefined-boundary
Open

emme1t wants to merge 1 commit into
chaijs:mainfrom
emme1t:fix/ordered-members-undefined-boundary

Conversation

@emme1t

@emme1t emme1t commented Sep 12, 2026

Copy link
Copy Markdown

Ordered member inclusion currently accepts expected sequences that extend past the target with undefined values. For example, expect([1]).to.include.ordered.members([1, undefined]) passes because reading past the target's end produces undefined.

Check the sequence lengths before ordered comparison. Regression coverage exercises strict and deep comparisons through expect, should, and assert, including empty targets, trailing undefined values, valid equal-length matches, and negation.

Validation on Node.js v24.16.0 (Windows):

  • Before the fix: all 8 affected test cases fail with the added regressions.
  • After the fix: all 8 affected test cases pass.
  • npm run test-node -- --reporter dot: 511 passing.
  • npm run lint: passes with the existing JSDoc warnings.
  • Browser suite: 510 passing on Chromium 151.0.7922.34, using the repository's browser configuration with a local executable path override.

Developed with assistance from OpenAI Codex; reproduction and tests were run locally.

@emme1t
emme1t requested a review from a team as a code owner September 12, 2026 05:52

@lbesecker195 lbesecker195 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed the bug on main: isSubsetOf compares elem === superset[idx] for ordered subsets, so reading past the end of the target yields undefined and expect([1]).to.include.ordered.members([1, undefined]), expect([]).to.include.ordered.members([undefined]) and assert.includeOrderedMembers([1], [1, undefined]) all pass. On this branch they fail with the expected message, while [1, 2, 3] including [1, 2], [1, undefined] including [1, undefined], and [] including [] still pass. The non-include path already checks lengths, so the new guard only affects contains. After npm run build, the node suite is 511 passing. LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants