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

Fix Faker::Vehicle.vin #2562

Merged
merged 2 commits into from
Sep 17, 2022
Merged

Conversation

rmm5t
Copy link
Contributor

@rmm5t rmm5t commented Sep 14, 2022

Summary

/cc @koic @stefannibrasil

- Dashes aren't permitted in VINs.
- This is a partial revert and fix of d113aab
- See faker-ruby@d113aab#r83976559
Copy link
Contributor

@stefannibrasil stefannibrasil left a comment

Choose a reason for hiding this comment

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

@rmm5t Thank you!

What do you think of adding the following in the test_vin_validity?

100.times { refute_match(/[^a-zA-Z\d]/, @tester.vin) }

or a simpler check that refutes non-alphanumeric characters. I feel like this could help us catch any issues like this in the future.

@rmm5t
Copy link
Contributor Author

rmm5t commented Sep 15, 2022

@rmm5t Thank you!

What do you think of adding the following in the test_vin_validity?

100.times { refute_match(/[^a-zA-Z\d]/, @tester.vin) }

@stefannibrasil Sure thing. I added 100.times to the validity test. The refute_match test above isn't necessarily correct, because letters such as I and O and Q aren't allowed in VINs.

Copy link
Contributor

@stefannibrasil stefannibrasil left a comment

Choose a reason for hiding this comment

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

Awesome, thank you so much for improving the tests so we can catch those details 👍

@stefannibrasil
Copy link
Contributor

@rmm5t ahhh, I wasn't aware of these restrictions. Thank you!

@stefannibrasil stefannibrasil merged commit a4b077f into faker-ruby:master Sep 17, 2022
@rmm5t rmm5t deleted the fix-vehicle-vin branch September 18, 2022 00:05
sudeeptarlekar pushed a commit to sudeeptarlekar/faker that referenced this pull request Oct 7, 2022
* Fix Faker::Vehicle.vin

- Dashes aren't permitted in VINs.
- This is a partial revert and fix of d113aab
- See faker-ruby@d113aab#r83976559

* Improved Faker::Vehicle.vin validity test
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