Future request: Multi-threaded tests should be explicit

[kikito]

@DorianGray mentions in #24:

We want to eventually have threaded tests. I went ahead and broke the logic out into coroutines to separate the code, and also to learn coroutines! Hopefully lualanes will eventually replace the pcalls and the coroutines so that each describe block can exist in it's own thread. Then, the tests will only run as slow as the slowest test and not an addition of all the run-times of all the tests combined.

I have first and second hand experience with large installations with many thousands of tests in several languages and it kills developer productivity when tests take forever.

While I agree with the sentiment (slow tests = bad) I'm opening this issue to express my disagreement on the implementation.

I don't think buste should be "threaded by default". My main reason is that my tests usually require some kind of shared state, like so:

describe("A non-threadable test", function()

  local counter -- this variable is shared among all tests on this block

  before_each(function()
    counter = 0 -- always reset the shared state to a known state
  end)

  it("can increase the counter", function()
    counter = counter + 1
    assert.equals(1, counter)
  end)

  it("can decrease the counter", function()
    counter = counter - 1
    assert.equals(-1, counter)
  end)
end)

The test case above runs fine in single-threaded mode. Even if the order of the tests is randomized, it will still pass. But it can fail randomly if the two tests are run in parallel. This would surprise most people coming from other testing libraries.

Note that while in this case I'm using a local variable to represent shared state, in other cases it's an external element, such a library, over which I might not have full control of - more notably, it might not be multi-thread capable.

I'm not 100% sure of how other libraries deal with multi-threading, but I think the cautious thing would be using a separate syntax for those tests that could be run in paralel. For example via a new multi_threaded method:

describe("A multi-threaded test", function()
   function createCounter()
     return 0
   end

  multi_threaded(function()

    it("can increase the counter", function()
      counter = createCounter()
      assert.equals(1, counter)
    end)

    it("can decrease the counter", function()
      counter = createCounter()
      assert.equals(-1, counter)
    end)

  end)
end)

Something explicit like this will ensure that the programmer "declares that he knows where he's running into" , so multi-threading doesn't take him "by surprise".

[Tieske]

as mentioned in issue 24, I think that multithreading can be done at the testfile level. That would also be a perfect border for your case of depending tests. Or, also mentioned, at the top-level 'describe' functions, but in that case some documentation on this subject would be required.

[ajacksified]

Yeah; I think we've decided that the threading will be per-file, with a flag to disable threads.

[Tieske]

as a request; check on Lanes being installed first, if not run sequential, otherwise use Lanes (unless flagged not to)

[DorianGray]

I agree with the check idea. We should thread by default if lanes is installed, with an option to disable threading. If your tests break when threaded you're doing them wrong and deserve to have to do extra work to run your tests slower =)

[DorianGray]

I vehemently disagree with a separate syntax for threaded tests. It clutters things.

[Tieske]

if you spawn threads on the file level, then I don't see why it would be necessary at all to have a separate syntax. Keep shared state at the file level and document the approach to multithreading. Syntax can stay as is.

[neha-bathra]

Hello Mr. DorianGray

I want to know when to perform multithreaded testing? can you please shed some insight on the same?