modules required in one test are not reloaded in another test

[karlp]

I'm using the "uci" library in two different spec files, and in one of them I set the path in a setup() call. This works well, but the uci library is not reloaded when the second spec file is run, and the path remains changed as set in the first file.

I can save and restore the path in the teardown call, but I'd like each test (file) to be run in isolation if possible.

[ajacksified]

Hm. I suppose we could run describes in s setfenv to prevent leaks.

[Tieske]

Note: setfenv() has been deprecated in 5.2

[DorianGray]

You can set this up yourself in package.loaders as a workaround, you just need to invalidate the uci load and it will reload on next require. I am unsure if we should do this in busted, since it is expected lua behavior to have modules as singletons.

[DorianGray]

http://lua-users.org/wiki/TheEssenceOfLoadingCode - package.loaded stores the information about loaded packages, removing an entry from it will cause the affected module to be reloaded on next require.

[Tieske]

I think its this type of code that you mean?

Clearing the environment is one thing, to do it effectively, it would require sandboxing, anything less is hardly effective as eventually some module will showup that does more in the global namespace than only requiring itself to be reloaded.

So either don't do it (leave it up to the test author, but maybe document it), or go all the way providing a sandbox (still a question whether its a sandbox per testfile or per test)

[ajacksified]

I was thinking about a sandbox per describe, deep-cloning the sandbox from the upper-level describe, which would effectively create a sandbox per file. You'd also be able to sandbox parallel describes from each other.

I'm not sure that it'd be expected behavior to do it that way, though - so I'm also okay with the alternative, which is "do nothing, let Lua work as Lua does."

[Tieske]

Whatdo you mean with deep cloning? Come to think of it; If the setup and before_each handlers now cascade all the way up, it will generate a new environment automatically. Just cleanup needs to be arranged and requires must be in those handlers. Correct? I think it needs some more thought.

[ajacksified]

Yeah, I think you're right. The user should be responsible for cleaning up his module in the teardown function. I'd be ok with calling this closed.

[karlp]

In the other testing frameworks for other languages, this either works out of the box, or is available via an option, (fork=yes, with-isonlation, etc) If the only way to make this work is to fiddle with things in each describe()'s teardown, then could there be at least helper code to do that? and documentation on when/why you might want that?

Things like manipulating package.loaded might seem obvious to hardcore lua users and lovers of language internals, but I've gotten an awful lot done without even knowing about it. Adding tests that do what I expect shouldn't require me to learn those internal tricks. (Maybe it's not internal guts, but it sure feels like it to me)

[Tieske]

some code mimicking the setfenv behaviour, before running a test, could do that (creating a sandbox actually) the cascading setup/before_each will then setup the enviormnet before the test.
After the test the original environment needs to be restored then, and probably restoring the package loaders cached stuff.

still tricky.

[DorianGray]

I'm not sure what the right answer here is.

1. We could implement sandboxing at the describe level...but that makes child describes extremely complicated to make work correctly.
2. We could add a busted-specific require method that does not set package.loaded to true. I hate this because it's non-standard and will confuse a lot of people.
3. We could do nothing. I don't like this because this is an annoyance. As a developer, writing tests is tedious enough without having to manually set up and tear down other people's modules potentially. Then again, we should be writing modules that do not interfere with globals and that can be instanced.

Pick one, no matter what you pick one use case is difficult. I'm hoping someone can provide an option number 4.

[Tieske]

nr 4. add 2 commands eg. isolatebusted() and deisolatebusted(), or sandbox() and desandbox() whenever one of these is called in a setup/before_each, every test after that will be run in a sandbox (or it will stop doing that). So it is optional bahaviour. Maybe wrap it;

sandbox(it("tests stuff in a sandbox, function(
-- do some testing
end))

[ajacksified]

I like the sandbox idea.

[DorianGray]

So what if we just went through package.loaded and set everything to false? Maybe track what gets required in each describe and unload the differences after the describe runs?

[Tieske]

I still like the sandbox idea, but maybe the ˋpackage.loadedˋ table can be added to the luassert snapshots

[ajacksified]

I'm okay with either setting it to false using an option (as @karlp noted, other frameworks have flags like isolation) or using a sandbox. I think I'm leaning towards options, just so you can change functionality at test time instead of having to rewrite tests to change how the tester runs.

[Tieske]

still tricky. Just reverting package.loaded doesn't revert changes to other globals. For example the penlight modules usually have an export method which exports several functions as globals.

I think a sandbox is the only clean option. But what to sandbox? fileset level, file level, describe level or test level? it will have a performance impact I guess

[DorianGray]

Good news! This is now possible in it's entirety with a small patch to Busted context environments! I'll implement it soon if no one else beats me to it.