There should be at least an option to insulate every test implicitly

[peabnuts123]

From what I can tell, sandboxing in Busted works as follows:

1. Files are sandboxed by default (overridable with --no-auto-insulate)
2. Sandboxed blocks can be explicitly declared with the insulate syntax

As far as I'm aware, every other Unit testing framework I've used runs every test individually, as if they were invoked separately. Now I'm not suggesting that this project conform to other frameworks simply for the sake of consistency, but coupled with my related issue #534, the fact that I must explicitly setup/teardown each test is a lot of effort for something that should just be a command line option or similar.
My project contains a complex state that I set-up specifically for each test, then assert that an interaction works as expected. Every test needs to have its own configuration before asserting, and I can't "roll forward" with the same state as that will absolutely lead to fragile tests.
Ideally busted should have a command line option that would automatically treat each test as if it were insulated.

[DorianGray]

If I'm understanding you correctly, it works that way by default. Insulated is the default state.

[peabnuts123]

Insulation is on by default per file. From the docs:

By default each test file runs in a separate insulate block, which can be disabled with the --no-auto-insulate flag.

Tests within the same file completely interfere with one-another, as the save() and restore() functions (from busted/context.lua) are only called by default at the start/end of a file.
In my own project I've currently achieved the desired behaviour by copying the save() and restore() functions and explicitly invoking them myself from before_each() and after_each().

[DorianGray]

I believe in your case it's because the functions are defined in the parent scope, therefore writing to variables in the parent scope instead of each test scope. I think this is expected behavior for someone who understands lua's lexical scoping.

[ajacksified]

I agree - this works as expected with lua's lexical scoping. If you define a variable at a higher scope, I would think it would be unexpected to be able to access it from within a test.

As far as I'm aware, every other Unit testing framework I've used runs every test individually, as if they were invoked separately.

In mocha, the most popular node testing framework, for example - if you define a variable in the parent's scope, you can access it:

var counter = new Counter();

describe('countring',  () => {
  beforeEach(() => counter.add());

  it('starts at 1', () => {
    expect(counter.value()).to.equal(1);
  });

  it('goes to 2', () => {
    expect(counter.value()).to.equal(2);
  });
});

(a contrived example.)

rspec has special syntax for creating instance variables that are reset after a test has passed: http://betterspecs.org/#let

Maybe something like that would make sense, so you could explicitly create variables which have their states reset after each test?

[peabnuts123]

I understand lexical scoping very well. The biggest problem comes from how Lua caches loaded modules. Take a look at the following example:

Counter.lua

local Counter = {}

local count = 0

function Counter.Add()
    count = count + 1
end

function Counter.GetValue()
    return count
end

return Counter

Counter_spec.lua

describe("Counting", function()
    it("starts at 0", function()
        local Counter = require('Counter')

        -- True, Counter starts at 0
        assert.are_equal(0, Counter:GetValue()) 
    end)

    it("increments to 1", function()
        -- NOTE that this returns the cached package.loaded instance of Counter!
        local Counter = require('Counter')  
        Counter:Add()

        -- True, 0 incremented to 1
        assert.are_equal(1, Counter:GetValue()) 
    end)

    it("starts at 0 - again", function()
        -- NOTE that this returns the cached package.loaded instance of Counter!
        --  which is still at 1
        local Counter = require('Counter')  

        -- False, Counter is still at 1 from test B
        assert.are_equal(0, Counter:GetValue()) 
    end)
end)

A similarly contrived example, but a good demonstration as to how this non-insulation causes problems. Nothing about the way I've structured this code is "global", but because Lua will always return the same instance of a loaded module, it's equivalent to just requiring Counter at the top of the file and operating on that. It seems difficult to test a module like Counter without the tests interfering, short of creating an "Instance" or "Factory" pattern, which severely limits the potential of what Lua can do.

[ajacksified]

Nothing about the way I've structured this code is "global"

You are doing exactly that - creating a global. As you know how Lua modules work, you also know that you're either abusing the fact that they're cached to create a de-facto global (because if you have two files require counter, they will operate on the same, cached instance), or you've done it on accident, in which case your tests should be failing. In either case, your tests work exactly the same as the code which implements Counter.

short of creating an "Instance" or "Factory" pattern, which severely limits the potential of what Lua can do

This is how node works as well - so I'm also familiar with the problem. It's an antipattern to return any variable - table or otherwise - from a module. I'm not sure how this "severely limits the potential of what Lua can do", but a proper way to expose a Counter would be:

return function ()
  local Counter = {}

  local count = 0

  function Counter.Add()
      count = count + 1
  end

  function Counter.GetValue()
      return count
  end

  return Counter
end

This would then be imported and run as a function, which would return a new instance of Counter. You could then write your tests thus:

local Counter = require('Counter')

describe("Counting", function()
    local counter
    beforeEach(function ()
        counter = Counter()
    end)

    it("starts at 0", function()
        assert.are_equal(0, counter:GetValue()) 
    end)

    it("increments to 1", function()
        counter:Add()
        assert.are_equal(1, Counter:GetValue()) 
    end)

    it("starts at 0 - again", function()
        assert.are_equal(0, counter:GetValue()) 
    end)
end)

If you feel limited by not being able to abuse the module system to create a global, you could even write one:

counter_global.lua

local Counter = require('Counter')
return Counter()

[DorianGray]

You're right, however, in that busted's insulation does not insulate all parts of the language. We don't believe we should be monkey patching "require", for instance, to solve this. The same happens when you load luajit ffi modules and then reload them because of the test insulation...it causes problems because of reasons outside our control.

Running test files in separate lua states is a planned feature for performance reasons. I don't think it would be really feasible to run each test in a separate state.

[peabnuts123]

Bah. I don't really consider returning an object from a module an anti-pattern, it's how the language works. It seems a bit ignorant to label it an "abuse" of the module system. By changing Counter.lua to return a function that returns instances of Counter, you've created a factory. A factory only makes sense if you intend to have multiple instances of a thing throughout your project. For structures that should only have one instance (for example, a utility module that does something, say, writes file data to disk) it is a code smell to potentially have multiple instances of it lying around in memory.

@DorianGray You're right that it does not insulate all parts of the language, which is what I am referring to in #534. I can accept why it would be too difficult to fully patch/restore the state of the Lua environment, but I thought it would be worth highlighting. However, I have applied the CURRENT save/restore functionality to before/after tests and it is working well. This is all I'm suggesting in this Issue, as it would not interfere with the way anybody currently works, would be simple to implement, and is feasible to achieve technically.

I'm kind of just being told my code stinks. I could argue that my code is good but this is not the forum for that. I think this is a good, simple addition to busted that would take it from a good framework to a great framework, and I would like to have a discussion about why it should/shouldn't be included, and not whether this is a real issue or not.

Semi-relatedly, if I'm looking to contribute, is it expected to have ~80 failing Unit Tests? I checked-out /master and ran bash try and it's showing 81 failing tests, 3 errored. Am I doing that correctly?

[mpeterv]

It's an antipattern to return any variable - table or otherwise - from a module.

Do you mean this for node.js? In lua returning a table from a module is widely considered best practice.

[DorianGray]

I don't think it's simple, that's the problem. I would encourage you to create a pull request that implements exactly what you're thinking to get rid of whatever miscommunication is happening here.

[peabnuts123]

Semi-relatedly, if I'm looking to contribute, is it expected to have ~80 failing Unit Tests? I checked-out /master and ran bash try and it's showing 81 failing tests, 3 errored. Am I doing that correctly?

@DorianGray please advise?

[DorianGray]

They all pass on my machine and in travis.... I don't have enough information to advise.

[peabnuts123]

Alright, thanks. I must be missing some dependencies or similar

[Tieske]

For insulation it would also be necessary to deep-copy upon save instead of just the simple copy now implemented. Including all sub/metatables (recursively) etc. And, retaining the original table, on restore clean that one up, not creating a new one. This is a challenge in itself...