Skip to content

line-up: add test cases for ordinal numbers that are multiples of 11/12/13 and update the descriptions - #2672

Merged
glennj merged 3 commits into
exercism:mainfrom
QuentinNev:fix/add-line-up-ordinal-edge-cases
Aug 9, 2026
Merged

line-up: add test cases for ordinal numbers that are multiples of 11/12/13 and update the descriptions#2672
glennj merged 3 commits into
exercism:mainfrom
QuentinNev:fix/add-line-up-ordinal-edge-cases

Conversation

@QuentinNev

@QuentinNev QuentinNev commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

See discussion: http://forum.exercism.org/t/line-up-exercise-missing-test-case-for-ordinal-suffix-multiples-of-11-12-13-outside-the-teens-range-are-not-covered/67586

Summary

Adds test cases for line-up covering ordinal numbers that are multiples of 11, 12, or 13 outside the 11–13 range itself.
It also reformulate the descriptions to be more explicit about what it's testing.

Problem

Uncovered multiples of 11, 12 and 13

The test suite for line-up does not include any numbers whose last two digits are a multiple of 11, 12, or 13 outside the 11–13 range itself (e.g. 22, 24, 33, 36, 44, 52, 60, 63, 65, 72, 78, 91, 96…).

This gap allows an incorrect implementation to pass. For example, a common bug is checking divisibility instead of range when handling the "teens" exception:

if (last2 % 11 === 0 || last2 % 12 === 0 || last2 % 13 === 0) return `${number}th`;

This looks like it's meant to handle the 11th/12th/13th exception (and does, since 11%11=0, 12%12=0, 13%13=0), but it also incorrectly catches any other multiple of 11, 12, or 13 (such as 22, 33, 44, 52, 91) and returns "th" for them instead of the correct "nd"/"rd"/"st".

Unclear descriptions

As proposed in the discussion, the descriptions weren't clear enough about what they're covering

Changes

  • Added 5 new test cases: 22, 33, 44, 52, 91, covering both the exceptional (nd/rd/st) cases that expose the bug and one non-exceptional (th) case as a regression check.
  • Updated descriptions to specify if it's a multiple of 11, 12 or 13 and the expected suffix

@github-actions

This comment was marked as resolved.

@github-actions github-actions Bot closed this Jul 26, 2026
@kotp kotp reopened this Jul 27, 2026
@keiravillekode

Copy link
Copy Markdown
Contributor

As requested in the forum, one or more of the numbers should be greater than 255. For example, 891, 923 or 972

@IsaacG

IsaacG commented Jul 27, 2026

Copy link
Copy Markdown
Member

...and please make the descriptions more descriptive of the purpose of the tests (i.e. they should convey what the test adds/what gap they cover)

@QuentinNev QuentinNev changed the title line-up: add test cases for ordinal numbers that are multiples of 11/12/13 line-up: add test cases for ordinal numbers that are multiples of 11/12/13 and update the descriptions Jul 28, 2026
@QuentinNev
QuentinNev force-pushed the fix/add-line-up-ordinal-edge-cases branch from 550d34a to 2ed5484 Compare July 28, 2026 17:56
@IsaacG

IsaacG commented Jul 31, 2026

Copy link
Copy Markdown
Member

@QuentinNev Any updates here?

1 similar comment
@IsaacG

IsaacG commented Aug 6, 2026

Copy link
Copy Markdown
Member

@QuentinNev Any updates here?

@BethanyG BethanyG left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🎉

@glennj

glennj commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

The links that are failing are all from exercism.org, returning status 403. Safe to merge, I'd say

@glennj
glennj merged commit 03f8331 into exercism:main Aug 9, 2026
6 of 7 checks passed
},
{
"uuid": "3f6c408c-4331-42b6-bb6c-3ad0823e568a",
"description": "format non-exceptional ordinal numeral 72 ending in nd even though it is a multiple of 12",

@glennj glennj Aug 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hang on a sec: why is 72 non-exceptional? It's not different from 52 which is exceptional.

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.

8 participants