lunarmodules / busted

Elegant Lua unit testing.
https://lunarmodules.github.io/busted/
MIT License
1.4k stars 185 forks source link

Busted "busts" otherwise working Lua code #609

Closed alerque closed 5 years ago

alerque commented 5 years ago

I'm having a strange issue with Busted where 5 out of 38 tests are failing for me in a suite that a) works fine for others and b) used to work for me. I'm on a new fresh machine running 2.0.0, and it's possible that I was previously using an older version and/or that others are on older versions too. I'm looking into that.

Travis has been running this suite forever and not had a problem.

I can replicate almost exactly the same code in plain Lua:

SILE = require("core/sile")

local frame = SILE.newFrame({ id = "hello", top = 20, left = 30, bottom = 200, right = 300 })
print(frame:height())

Run from the console as lua test.lua the answer I get is 180, which is correct. However trying to do the same thing with Busted gives me a different outcome:

SILE = require("core/sile")

describe("The frame factory", function()

  describe("Simple", function()
    local frame = SILE.newFrame({ id = "hello", top = 20, left = 30, bottom = 200, right = 300 })
    it("should have height", function () assert.is.equal(180, frame:height()) end)
  end)

end)

Run from the console as busted test.lua I get this:

0 successes / 1 failure / 0 errors / 0 pending : 0.090093 seconds

Failure → tests/frame_spec.lua @ 7
The frame factory Simple should have height
tests/frame_spec.lua:7: Expected objects to be equal.
Passed in:
(number) 0
Expected:
(number) 180

I can inspect the frame object and and see that is has the necessary properties and height() function. But I can also run that function outside of the it() construct and it outputs 0. Oops.

I'm stumped on what could be wrong, what might have changed, or how to debug this further.

DorianGray commented 5 years ago

If the code you're testing uses globals busted 2.0 may have exposed a problem in that code that busted 1 would not have. You can attempt disabling sandboxing as documented at https://olivinelabs.com/busted/#defining-tests to see if that fixes the problem, but I'd suggest fixing the code to not use globals instead just for maintainability's sake.

alerque commented 5 years ago

Thanks for the idea @DorianGray. That sounded like a promising lead because there are issues with globals in this code we're still rooting out.

Sadly this doesn't seem to be the root of the problem. I just found that our CI builds in Travis are also installing the 2.0.0 Luarock for Busted, and it is passing all tests. At the latest I expect other so be using the RC13 version and there doesn't seem to be any changes between that and the final release that would affect this.

I have tried changing up the test locally using expose() (and out of curiosity with insulate() and inspecting the output manually outside of the test framework entirely, just running it using busted ... as well), but am getting the same failures in all cases.

DorianGray commented 5 years ago

It must be something else about your environment specific to the tests then. If the versions of busted are the same on both systems and one passes and another fails then it's the system

alerque commented 5 years ago

As predicted by logic (@DorianGray) this was something else on my system. It took a maddening process of elimination because Lua path handling is so weird. Basically the code being tested was pre-pending package.path with local paths to test, and specific versions of some Lua Rocks were being placed locally for testing consistency. This would have worked, except in the Busted test an environment variable and convoluted code was hiding the fact that the system's default Lua path was getting prepended at the end of the process (so the system path was now listed twice, and, more relevantly, first.

An old version of one of the Lua Rocks on the system(s) in question was causing this inconsistency.