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: A new test case with odd number of digits #1543

Merged
merged 2 commits into from
Oct 25, 2021
Merged

Luhn: A new test case with odd number of digits #1543

merged 2 commits into from
Oct 25, 2021

Conversation

fahadhk
Copy link
Contributor

@fahadhk fahadhk commented Jun 24, 2021

A new positive test case with odd number of digits and non-zero first digit.
I have seen a solution in Golang which passes all the tests for Luhn containing odd number of digits and never reads the first digit.
It happens because in all such cases the first digit is zero . I am sure it is also relevant for C#.

@ErikSchierboom
Copy link
Member

@fahadhk The test cases that we use in the luhn exercise are generated from shared metadata found in https://github.com/exercism/problem-specifications/blob/main/exercises/luhn/canonical-data.json. Could you submit a PR there instead of to this repo? That way other tracks can also benefit from this new test case and we can just regenerate the exercise.

@yzAlvin yzAlvin mentioned this pull request Aug 19, 2021
@ErikSchierboom
Copy link
Member

This PR has been open for quite a while. Are you still interested in working on this @fahadhk?

@fahadhk
Copy link
Contributor Author

fahadhk commented Sep 19, 2021

@ErikSchierboom Pardon I was inactive for a while. Let me add a PR to the specification repo. I will post it here once it is done.

Thank you for pointing to that place.

Cheers

@fahadhk
Copy link
Contributor Author

fahadhk commented Sep 19, 2021

@ErikSchierboom PR#1845

Cheers

@ErikSchierboom
Copy link
Member

@fahadhk Thanks! I'll update once that PR is merged.

@ErikSchierboom
Copy link
Member

@fahadhk The prob-specs PR has been merged. This PR can now be updated. To do so, please do the following:

  • Rebase this branch on the latest main
  • Run ./bin/fetch-configlet or pwsh ./bin/fetch-configlet.ps1
  • Run ./bin/configlet sync --exercise luhn
  • Open a prompt in the generators directory and run dotnet run --exercise luhn
  • Commit and push

This should update the luhn exercise's tests and tests.toml file.

@ErikSchierboom
Copy link
Member

@fahadhk Small bump. See my comment above for instructions on how to proceed.

A new positive test case with odd number of digits and non-zero first digit.
A new positive test case with odd number of digits and non-zero first digit.
@fahadhk
Copy link
Contributor Author

fahadhk commented Oct 24, 2021

@ErikSchierboom Sorry I was away. Let me do it now.

I have a updated tests.toml and test file with the test that I proposed by skipping others.

Looking forward to hear from you.

P.S. Meanwhile bumped in to a problem of missing DLL while running Generator on ubuntu 20.04 LTS, solved it using

libgit2/libgit2sharp#1747 (comment)

@fahadhk
Copy link
Contributor Author

fahadhk commented Oct 24, 2021

$ ./bin/configlet sync --exercise luhn
Checking exercises...                                                               
Cloning https://github.com/exercism/problem-specifications/... success
[warn] luhn: missing 3 test cases                                                   
       - very long input is valid (b9887ee8-8337-46c5-bc45-3bcab51bc36f)
       - valid luhn with an odd number of digits and non zero first digit (8a7c0e24-85ea-4154-9cf1-c2db90eabc08)
       - non-numeric, non-space char in the middle with a sum that's divisible by 10 isn't allowed (8b72ad26-c8be-49a2-b99c-bcc3bf631b33)
[warn] some exercises are missing test cases

Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

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

Great, thanks!

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