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

Bug: Returns non-string binaries when called on Emojis #34

Open
sheharyarn opened this issue Jul 12, 2019 · 2 comments
Open

Bug: Returns non-string binaries when called on Emojis #34

sheharyarn opened this issue Jul 12, 2019 · 2 comments

Comments

@sheharyarn
Copy link

This returns invalid string binaries when the string contains an emoji:

Slugger.slugify_downcase("Hi!")
#  => "hi"

Slugger.slugify_downcase("Hi! 👋")
# => <<104, 105, 45, 159, 145, 139>>
@sheharyarn
Copy link
Author

sheharyarn commented Jul 12, 2019

A related issue is that in the tests you check if the conversion is a binary (which it always will be), but don't check if the converted string is actually valid:

assert is_binary Slugger.slugify(string)
assert is_binary Slugger.slugify_downcase(string)

You should instead use:

assert String.valid? Slugger.slugify(string)
assert String.valid? Slugger.slugify_downcase(string)

Because then it will fail the tests:

 ▲ code/elixir/slugger mix test                                                                                   5s  master :: 2d :: ● :: ⬢
....

  1) test naughty strings (SluggerTest)
     test/slugger_test.exs:76
     Expected truthy, got false
     code: assert String.valid?(Slugger.slugify(string))
     arguments:

         # 1
         <<194, 173, 216, 128, 216, 129, 216, 130, 216, 131, 216, 132, 216, 133,
           216, 156, 219, 157, 220, 143, 225, 160, 142, 226, 128, 139, 226, 128,
           140, 226, 128, 141, 226, 128, 142, 226, 128, 143, 226, 128, 170, 226,
           128, 171, 226, 128, 172, 226, 128, 173, ...>>

     stacktrace:
       test/slugger_test.exs:81: anonymous fn/1 in SluggerTest."test naughty strings"/1
       (elixir) lib/enum.ex:783: Enum."-each/2-lists^foreach/1-0-"/2
       (elixir) lib/enum.ex:783: Enum.each/2
       test/slugger_test.exs:80: (test)

............................

Finished in 0.1 seconds
10 doctests, 23 tests, 1 failure

@sheharyarn
Copy link
Author

sheharyarn commented Jul 12, 2019

You can replace your test with this so it logs the exact string for which it fails in the big-list-of-naughty-strings.json file (will log only if the test for that specific string fails):

  describe "naughty strings" do
    "test/big-list-of-naughty-strings/blns.json"
    |> File.read!
    |> Poison.decode!
    |> Enum.with_index
    |> Enum.each(fn {string, index} ->
      @index index
      @string string
      @tag capture_log: true
      test "no. #{@index}" do
        Logger.debug("Testing: #{@string}")
        assert String.valid? Slugger.slugify(@string)
        assert String.valid? Slugger.slugify_downcase(@string)
      end
    end)
  end

[...]
 29) test naughty strings no. 189 (SluggerTest)
     test/slugger_test.exs:86
     Expected truthy, got false
     code: assert String.valid?(Slugger.slugify(@string))
     arguments:

         # 1
         <<157, 147, 163, 45, 157, 147, 177, 45, 157, 147, 174, 45, 157, 147,
           186, 45, 157, 147, 190, 45, 157, 147, 178, 45, 157, 147, 172, 45,
           157, 147, 180, 45, 157, 147, 171, 45, 157, 147, 187, 45, 157, 147,
           184, 45, 157, 148, 128, 45, 157, 147, ...>>

     stacktrace:
       test/slugger_test.exs:88: (test)

     The following output was logged:

     21:12:58.636 [debug] Testing: 𝓣𝓱𝓮 𝓺𝓾𝓲𝓬𝓴 𝓫𝓻𝓸𝔀𝓷 𝓯𝓸𝔁 𝓳𝓾𝓶𝓹𝓼 𝓸𝓿𝓮𝓻 𝓽𝓱𝓮 𝓵𝓪𝔃𝔂 𝓭𝓸𝓰

.

Finished in 1.6 seconds
10 doctests, 529 tests, 29 failures

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

1 participant