LPGhatguy / lemur

Partial implementation of Roblox API in Lua
MIT License
62 stars 25 forks source link

Provide cleaner way to clear the habitat loaded module cache #176

Closed DeanPhillipsOKC closed 5 years ago

DeanPhillipsOKC commented 5 years ago

Hello!

It seems Lemur caches module scripts loaded via the habitat implementation of the "require" method. This is understandable as it emulates the expected Lua behavior.

However, for unit test purposes it would be nice to have an easy way to clear this that is built right into the library.

Rationale When unit testing, it's often useful to start certain test cases from a clean slate which is clunky given Lua's (and therefore Lemur's) caching of module scripts.

Example

Consider the following Rodux dispatcher

local FishBagDispatcher = {}
FishBagDispatcher.prototype = {}

local dependencies = require(script.Dependencies).Get()

function FishBagDispatcher.Dispatch(totalFishCaught, bagContents)
    dependencies.VM:dispatch({
        type = "fishBagContentsChanged",
        FishBag = {
            Total = totalFishCaught,
            Contents = bagContents
        }
    })
end

dependencies.FishBagContentsChangedRE.OnClientEvent:Connect(function(newTotalFishCaught, newFishBagContents)
    FishBagDispatcher.Dispatch(newTotalFishCaught, newFishBagContents)
end)

local totalFishCaught = dependencies.GetTotalFishCaughtRF:InvokeServer()
local fishBagContents = dependencies.GetFishBagContentsRF:InvokeServer()

FishBagDispatcher.Dispatch(totalFishCaught, fishBagContents)

return FishBagDispatcher.prototype

I tend to like my unit tests as small, and focused as possible. Therefore, I would like to show that the total fish caught gets dispatched during loading in one test case, and the contents of the fish bag in another. Because the module script is cached after the first test case, I need to use a hack to clear the cache for the script. Though it's a one liner (see setup function below), it's not very expressive, and accessing the metatable just feels gross.

        local function setup()
            getmetatable(script.Parent).instance.moduleLoaded = false

            require(script.Parent.Dependencies).Inject(dependencies)

            dependencies.GetTotalFishCaughtRF.OnServerInvoke = function()
                return totalFishOnServer
            end

            dependencies.GetFishBagContentsRF.OnServerInvoke = function()
                return fishBagContentsOnServer
            end

            dependencies.VM.dispatchedObjectCapture = nil
        end

        it("Should retrieve the total fish caught from the server when loading.", function()
            totalFishOnServer = 11
            fishBagContentsOnServer = nil

            setup()

            require(script.Parent)

            expect(dependencies.VM.dispatchedObjectCapture.FishBag.Total).to.equal(11)
        end)

        it("Should retrieve the contents of the fish bag from the server when loading.", function()
            totalFishOnServer = nil
            fishBagContentsOnServer = {
                StarFish = 2,
                SunFish = 4
            }

            setup()

            require(script.Parent)

            local dispatchedObj = dependencies.VM.dispatchedObjectCapture.FishBag.Contents

            expect(dispatchedObj).to.be.ok()
            expect(dispatchedObj.StarFish).to.equal(2)
            expect(dispatchedObj.SunFish).to.equal(4)
        end)

Would you be open to a nicer way to solve this? A few ideas are...

Kampfkarren commented 5 years ago

I have this module in my testing suite:

local function RequireWithoutCache(object)
    local result = require(object)
    getmetatable(object).instance.moduleLoaded = false
    return result
end

return RequireWithoutCache
DeanPhillipsOKC commented 5 years ago

Thanks Kampfkarren. The solution that both you, and I came up with requires examination of source code internals, and is a hack that exploits lax encapsulation. Since starting from a clean state is a pretty essential feature for unit testing, it seems like direct, expressive library support would be nice.

Now that I think of it though, do you think this is more an issue that should have been created on the TestEZ repository, since the use case would only be relevant to unit testing, and not necessarily the concern of Lemur?

Kampfkarren commented 5 years ago

TestEZ is meant to be run in real Roblox, so I'm not sure how that'd work.

In real Roblox you can cheat the cache by using require(module:Clone()), but Lemur doesn't support Clone yet.

I'm not exactly too sure, but I'd think this is a Lemur issue.

LPGhatguy commented 5 years ago

I'm mostly against the idea of extending Lemur or TestEZ to let it support doing things that aren't supported in Roblox. The goal of both projects first and foremost is to allow a faithful testing environment for continuous integration for Roblox, and any divergence hurts that goal!

I think what Kampfkarren suggested -- cloning your module -- is probably the best way to test things that absolutely need module-level side effects or state. In general though, I'd advise not having side effects or mutable module state, because they make testing more difficult and sort of obscure what require does at any given point.

DeanPhillipsOKC commented 5 years ago

I see where you're coming from. Require is essentially a method call and my uncaching the call is not a hack because of a missing feature in the library as I originally stated, but rather a hack to work around a design / implementation flaw (i.e. the module script was written naively, and resulted in the call to require not being idempotent).

If that's what you meant, I understand and will update my middle scripts.

Did not really think of require as a method call that should be held to the same functional programming standards as any other method.

Thanks for the advice and sorry about another issue that's anything but.

LPGhatguy commented 5 years ago

Don't worry about filing issues that don't turn into real issues! It's always great to get feedback on the tools and libraries I work on, regardless of the outcome. Even misunderstandings can point to bad documentation or a slightly off-kilter design. 😄