search

lunarmodules / Penlight

A set of pure Lua libraries focusing on input data handling (such as reading configuration files), functional programming (such as map, reduce, placeholder expressions,etc), and OS path management. Much of the functionality is inspired by the Python standard libraries.
https://lunarmodules.github.io/Penlight/
MIT License
1.89k stars 239 forks source link

Isolate testing Penlight with busted from Busted using Penlight #363

Open alerque opened 3 years ago

alerque commented 3 years ago

So my brilliant idea in #356 to use Busted for the test suite has an unexpected consequence! I just spent an hour chasing my tail trying to run tests and getting bizarre results before it finally dawned on my why it was breaking in places I hadn't touched. Busted itself requires Penlight for internal use. :facepalm:

The result of course is a dependency race condition. It's not always clear what version of Penlight is being used where! It works okay testing Penlight modules that Busted has not already loaded, but even manually setting the package.path is useless for testing modules that Busted already loaded for its own use, all you get is the cached version! Conversely it's possible to convince Busted to load the local code, but then a broken module can actually break the Busted runner itself rather than failing the respective test.

I'm not quite sure how to untangle this so that both local and CI test runs reliably ⓐ use a stable version of Penlight to run Busted and ⓑ test the Penlight module code as found in the local working directory.

alerque commented 3 years ago

It seems like Busted's insulate() and particular the --auto-insulate which defaults to on should be taking care of this. I'm kind of confused why it is not, this could be a Busted bug.

Tieske commented 3 years ago

insulation will revert the loaded modules to the ones that were loaded at the start of the test I think. So libs loaded during the test will be cleared for the next one. But libs loaded before the tests started will not be removed before a test starts.

The workaround would be to iterate over package.loaded and set every entry matching pl.* to nil. That assumes Busted has already loaded all its modules (otherwise if you call into some busted function during a test, like finally, and it does a require, it will load the PL module to be tested instead of the one Busted was installed with).

This exact behaviour saved us from the LuaJIT ffi segfaults, because we could load the ffi before the tests started and it would not be cleaned by busted anymore (GC'ing that module will lead to segfaults).

Tieske commented 3 years ago

discussion moved to https://github.com/Olivine-Labs/busted/issues/643

alerque commented 3 years ago

The workaround would be to iterate over package.loaded and set every entry matching pl.* to nil.

I've tried several ways of doing just this but haven't come up with a way to do it pervasively without side effects that break other Busted stuff.