Line Up Exercise - Missing test case for ordinal suffix, multiples of 11/12/13 outside the teens range are not covered

Exercism Building Exercism
1

Description

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”.

Expected behavior

  • 22 → "22nd" (a buggy implementation using the check above would produce "22th")
  • 33 → "33rd"
  • 52 → "52nd"
  • 91 → "91st"

Suggested fix

Add at least one test case per suffix category (1st/2nd/3rd/4th-equivalents) that is also a multiple of 11, 12, or 13, to catch this class of bug (e.g. 22, 33, 52, 91.)

If line-up has canonical data in problem-specifications, it may be worth raising there too, since it’s shared across tracks.

2

Did you meant to raise this for a specific track? Is there a way to determine if there’s data there?

3

I think that’s a common mistake to make. +1 for covering the gap.

@QuentinNev Can you suggest concrete test cases (problem spec JSON structured) for further discussion?

4

For the JSON structure, see canonical-data.json

If you have cloned a typical track repo, here is a convenient way to generate a UUID for a test case:

bin/fetch-configlet
bin/configlet uuid
5

@IsaacG I raised it for the TypeScript track specifically, since that’s where I hit the bug.

@mk-mxp Here’s some tests that cover the missing edge cases.

{
  "uuid": "a98e2e22-ab41-4557-a7c2-efedc19c16da",
  "description": "format exceptional ordinal numeral 22",
  "property": "format",
  "input": {
    "name": "Ingrid",
    "number": 22
  },
  "expected": "Ingrid, you are the 22nd customer we serve today. Thank you!"
},
{
  "uuid": "ab45d2fb-e0ee-4016-b605-76917584db0a",
  "description": "format exceptional ordinal numeral 33",
  "property": "format",
  "input": {
    "name": "Mario",
    "number": 33
  },
  "expected": "Mario, you are the 33rd customer we serve today. Thank you!"
},
{
  "uuid": "3f6c408c-4331-42b6-bb6c-3ad0823e568a",
  "description": "format non-exceptional ordinal numeral 44",
  "property": "format",
  "input": {
    "name": "Ugo",
    "number": 44
  },
  "expected": "Ugo, you are the 44th customer we serve today. Thank you!"
},
{
  "uuid": "c9243603-9f17-45b3-9a41-db9ebdbf08e1",
  "description": "format exceptional ordinal numeral 52",
  "property": "format",
  "input": {
    "name": "Quentin",
    "number": 52
  },
  "expected": "Quentin, you are the 52nd customer we serve today. Thank you!"
},
{
  "uuid": "8db52cd9-9689-413f-a812-6c36fcfd0d07",
  "description": "format exceptional ordinal numeral 91",
  "property": "format",
  "input": {
    "name": "Boris",
    "number": 91
  },
  "expected": "Boris, you are the 91st customer we serve today. Thank you!"
}

@keiraville Thank you!

Should I have opened a pull request on the repo instead ?

6

This is the right place. Wait for three +1 votes from maintainers before creating a problem-specifications PR.

7

There is a Typescript category on the forum for Typescript specific issues. Your post wasn’t in that category nor did it mention Typescript :slightly_smiling_face:

But if you check the problem specs, you can see if an exercise comes from there or not. If it does, then any test gaps should be filed against that and not the individual track.

8

The test descriptions mention “exceptional” and “non exceptional” values. Can you make those more descriptive? Reading the tests, I can’t tell what makes one number any more exceptional than another and what is exceptional about the exceptional cases.

9

From what I recall from the original PR, an exceptional number was one not ending in “-th”. The new test descriptions seem to match this convention. I agree it’s confusing, but I think a test description rename should be done across the board with reimplemented test cases. Given the noise that’ll create, I’d rather not do that. Maybe we can change the instructions to make it clear that the “-th” rule is the default and the other rules are exceptions to it. Either solution (reimplementing the previous test cases or updating the docs feels out of of scope for this PR / thread and can be handled separately.

10

Independent of the prior tests, new tests should have clear descriptions. I don’t think new tests need to be consistent with the prior descriptions here.

1 Like
11

I’m +1 for the change. I believe we should have at least one multiple of each, 11, 12, and 13, that does not end in th.

Also, given that the instructions say numbers are up to 999, but current test cases are at most 123, we could have at least one larger multiple. A number greater than 255 is particularly interesting because it doesn’t fit into a single byte.

1 Like
12

+1 from me as well. And as @oxe-b said, some large numbers would be fun.

2 Likes
13

Looks like you got another +1, and as I saw these coming in via e-mail, I also noticed the exercism/problem-specifications#2672 come up.

I will see if I can add the link to this thread to complete the communication loop, but in case I can not, can you double check that it has happened @QuentinNev.

2 Likes
14

The test description of the new test cases should state, that they are multiples of 11/12/13. That would reflect why they are not just repeated exceptional / non-exceptional numbers.

2 Likes
15

Yeah, I’m a +1 for the original proposed tests plus the multiples tests proposed by by oxe-b and mk-mxp. Like Isaac noted, we should have more descriptive test names for these new tests. For the multiples tests, I like the simplicity of mk-mxp’s suggestion for the test description. For the other tests we’re adding, exceptional vs. non-exceptional is vague. Perhaps we could describe the result? “format exceptional ordinal numeral 91” becomes “format large number ending in -st”.

16

Thanks for the feedback. I’ve updated the PR with the following changes:

  • Added 1 test case with large number
  • Updated all descriptions to specify when a number is a multiple of 11, 12 or 13 and the expected suffix
17

The expected suffix is pretty clear from the expected output :slightly_smiling_face:
Do the descriptions explain why the test exists or what gap they are intended to cover?

1 Like
18

You’re right, they’re really repetitive now and some of the tests cover the same cases.
Should we get rid of tests covering 4-9 ?

19

Testing 1-10 is probably good to have just to have the base cases covered.

Let’s focus on the newly proposed tests and not mess with the existing tests. Unless you’d like to start over with a brand new proposal!

1 Like
20

Honestly I don’t really know. I’ve applied all suggestions at once but I’m not sure what is expected anymore and I’ve just been procrastinating, paralyzed by doubt ever since.

It’s the first time that I’ve actually contributed to something where my work is being seriously reviewed.

The descriptions I work are fine, I guess ? Should I just remove the suffix then ? But if it’s pretty clear from the expected output, we can apply the same to the number so almost all description become : “format number” but as the goal of the exercise to format number, we could juste write nothing because everything is just implicit from the context, input and output.

That’s why I’m stuck, I just don’t know anymore.