-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Faker::Internet.user_name can't handle UTF-8 arguments #1421
Conversation
@vbrazo, would you know why CI is failing for ruby 2.4.4 and 2.5.1? The patch seems to work correctly locally for both versions. |
@@ -48,6 +48,10 @@ def test_username_with_integer_arg | |||
end | |||
end | |||
|
|||
def test_username_with_utf_8_arg | |||
assert @tester.username('Łucja').match('Łucja') | |||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your solution looks good. What's killing you is the .downcase
at the end of the shuffle method: return shuffle(specifier.scan(/[[:word:]]+/)).join(sample(separators)).downcase if specifier.respond_to?(:scan)
From: /faker-ruby/faker/test/test_faker_internet.rb @ line 53 TestFakerInternet#test_username_with_utf_8_arg:
51: def test_username_with_utf_8_arg
52: require 'pry'
=> 53: binding.pry
54: assert @tester.username('Łucja').match('Łucja')
55: end
[1] pry(#<TestFakerInternet>)> @tester.username('Łucja')
=> "łucja"
Now Let me investigate a bit more. |
@ivanoblomov I know what it is. Ruby Let me know what you think. |
Nice catch @vbrazo, thanks! |
* test: covered bug * fix: passed * fix: typo * Minor fix * Fix test_username_with_utf_8_arg - Faker::Internet.user_name * Add comment above test_username_with_utf_8_arg * Update test_faker_internet.rb
Because the
\w
character class is exactly equivalent to[A-Za-z0-9_]
, we need[[:word:]]
to support UTF-8.Fixes #168.