lunarmodules / luassert

Assertion library for Lua
MIT License
202 stars 76 forks source link

The extract_keys method may fail if the token has three or more words #172

Open Triple-Z opened 3 years ago

Triple-Z commented 3 years ago

https://github.com/Olivine-Labs/luassert/blob/36fc3af9621696a9dfcf71c0bcd25cdbc9475cf8/src/util.lua#L322-L329

At line 328, the longkey maybe wrong in further iterations.

For example: if the tokens is {'is', 'match', 'error'}, the longkey variable will be match_match_error at the second iteration in the inner while loop. So it may fail to get the longest matching key when the key has three or more words.

Here is the potential fix which I brought up:

-      longkey = (token .. '_' .. key)
+      longkey = i > 1 and (tokens[i-1] .. '_' .. key) or key
Tieske commented 3 years ago

a failing test case would be nice

Tieske commented 3 years ago

tried this;

  it("multiple section modifiers #only", function()
    assert.is_not_is_not_equal(1, 1)
    assert.not_is_not_is_equal(1, 1)
    assert.not_not_is_is_equal(1, 1)
    assert.is_is_not_not_equal(1, 1)
    assert.is_not_is_not_is_not_equal(1, 2)
    assert.not_is_not_is_not_is_equal(1, 2)
    assert.not_not_not_is_is_is_equal(1, 2)
    assert.is_is_is_not_not_not_equal(1, 2)
  end)

But that passes nicely, so cannot reproduce this. Am I missing something @Triple-Z ?

Triple-Z commented 3 years ago

@Tieske Sorry, I consider this issue is created by assertions but no modifiers.

Here is my test for this:

it("three words token #only", function()
  assert:register("assertion", "three_words_token", function() print("can you see me?") return false end, "assertion.same.positive", "assertion.same.negative")
  assert:register("assertion", "more_three_words_token", function() print("can you see me now?") return false end, "assertion.same.positive", "assertion.same.negative")

  assert.is_not_more_three_words_token()
end)

The luassert will raise an error: "luassert: unknown modifier/assertion: 'is_not_more" for this test.


With the change which I proposed before, luassert will recognize the longer token and match it first.

Like this test:

it("three words token #only", function()
  assert:register("assertion", "three_words_token", function() print("you can't see me") return false end, "assertion.same.positive", "assertion.same.negative")
  assert:register("assertion", "than_three_words_token", function() print("you can't see me") return false end, "assertion.same.positive", "assertion.same.negative")
  assert:register("assertion", "more_than_three_words_token", function() print("you can see me") return false end, "assertion.same.positive", "assertion.same.negative")

  assert.is_not_more_than_three_words_token()
end)

However, I find there is another bug, the luassert cannot find the longest token if the tokens are not continuous.

Like this:

it("three words token #only", function()
  assert:register("assertion", "three_words_token", function() print("you can't see me") return false end, "assertion.same.positive", "assertion.same.negative")
  -- assert:register("assertion", "than_three_words_token", function() print("you can't see me") return false end, "assertion.same.positive", "assertion.same.negative")
  assert:register("assertion", "more_than_three_words_token", function() print("you can see me") return false end, "assertion.same.positive", "assertion.same.negative")

  assert.is_not_more_than_three_words_token()
end)

It would raise an error: "luassert: unknown modifier/assertion: 'is_not_more_than'" .

Tieske commented 3 years ago

tbh, I think the best route would be to document that tokens can have a maximum of 3 words. That is fair enough imo, and prevents us from going down a rabbit hole with this one.

wdyt @ajacksified @DorianGray ?

redcatbear commented 2 years ago

I just ran into the same problem, but in a little bit more surprising setup. Executing a spec with require "busted.runner"() directly worked just fine with a three-word-assertion. Running the same code from the console with busted gave me the "unknown modifier/assertion" error. Thanks to this thread I was able to shorten the assertion and circumvent the problem. Still quite the pitfall.