Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

luhn: expectation/description mismatch in canonical test data #1396

Closed
andeemarks opened this issue Nov 15, 2018 · 2 comments
Closed

luhn: expectation/description mismatch in canonical test data #1396

andeemarks opened this issue Nov 15, 2018 · 2 comments

Comments

@andeemarks
Copy link

andeemarks commented Nov 15, 2018

In problem-specifications/exercises/luhn/canonical-data.json, the following test spec has a mismatch between the stated expectation and the test description

    {
      "description": "a simple valid SIN that becomes invalid if reversed",
      "property": "valid",
      "input": {
        "value": "59"
      },
      "expected": true
    },

Given the value (a valid luhn number), the description should be changed.

History

In the exercism/kotlin@11faba6 commit, these tests had two expectations and better matched the descriptions (testing both the original and reversed state), but this intent was lost when the tests were matched against the canonical test data in commit exercism/kotlin@a2bb40e

Problem

The JSON schema doesn't seem to test setup described above where ideally two tests are needed to cover the reversal of the test data.

@petertseng
Copy link
Member

petertseng commented Nov 16, 2018

has a mismatch between the stated expectation and the test description

I don't agree that there is a mismatch.

The reason I say this is because the input "59" is "a simple valid SIN that becomes invalid if reversed", which is exactly as the description says. Nothing in the description indicates that "95" needs to be tested, because "95" would be e.g. "an invalid SIN that becomes valid if reversed".

I would argue that based on #547 (review) the purpose of that test is to see whether the implementation is selecting the correct set of digits to be doubled. Testing "95" adds no additional value to the test because the implementation that selected the wrong set of digits would either give the correct answer on both "59" and "95" or the wrong answer for both. Thus, only one needs to be tested.

Given the value (a valid luhn number), the description should be changed.

I neither agree nor disagree with this. I advise that my comment should not be interpreted to be an argument either for or against changing the description.

The JSON schema doesn't seem to test setup described above where ideally two tests are needed

This is true.

@rpottsoh
Copy link
Member

This is issue isn't driving towards a plan of action. Feel free to re-open if there is need to further discussion.

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

No branches or pull requests

3 participants