Designing to maximize testability - is this stupid?

Help and Feedback Code Review
,
#1

For the past few months I’ve been exploring how to automate testing for 100% of my code, even the parts that are heavily integrated with other modules. At some point I realized making just about every module a class (sorry, anti-OOP friends) would give me the ability to inject any dependency I like. This definitely opens the door to extremely thorough testing, but it comes at the cost of a structure that I find both pleasing and a gigantic pain to write.

Pros:

  • I love that I can inject dependencies to test heavily integrated code without having to resort to a pile of optional arguments in the constructor
  • I like the separation of construction (in new) and the startup (in init) that’s forced by the need to build the object and inject dependencies into it before any logic runs. As a happy accident this allows two services to depend on each other since they’re both initialized by a startup script.
  • Have I mentioned that I love that I can inject dependencies?

Cons:

  • Constructors are going to get very bloated with all of the private variables
  • Having to write self._* for so many private variables is going to get very annoying very fast

Overall I really do prefer this style of coding, but I’d like to hear other opinions. I’ve included an example module and test below.

InterfaceService.lua

Note: Roact doesn’t actually need to be put into the private variable since it’s effectively stateless for this use case and won’t need to be injected. I only put it there as a demonstration of how there will be situations where it would be necessary, e.g. a Rodux store where the state needs to be manipulated by the test.

local Players = game:GetService("Players")
local ReplicatedStorage = game:GetService("ReplicatedStorage")

local packages = ReplicatedStorage.Packages
local Roact = require(packages.Roact)

local components = ReplicatedStorage.Components
local App = require(components.App)

local localPlayer = Players.LocalPlayer

local InterfaceService = {}
InterfaceService.__index = InterfaceService

function InterfaceService.new()
	local self = setmetatable({
		_roact = Roact;
		_app = App.App;
		_appHandle = nil;
		_target = nil;
	}, InterfaceService)

	return self
end

function InterfaceService:init()
	self._target = self._target or localPlayer:WaitForChild("PlayerGui")
	local interface = self._roact.createElement(self._app)
	self._appHandle = self._roact.mount(interface, self._target)
end

return InterfaceService.new()

InterfaceService.spec.lua

This is where the magic happens. I’m able to inject a fake app to simplify the test and save on performance. This also means any of the UI components the InterfaceService normally uses won’t be tested either, which helps with compartmentalization (InterfaceService itself should not be failing when another module is the culprit.) But most importantly, I’m able to inject a fake target in place of the PlayerGui, which doesn’t exist on the server. This test would be near impossible to automate without being able to do this.

One odd bit is the way InterfaceService is constructed - InterfaceService.lua returns a pre-constructed service, not a class, but thanks to a quirk of metatables every class object contains the constructor that made it. This exposes the constructor to us, which we can grab and use to build a unique service exclusively for each test. The advantage of using unique service objects is that we can build and test as many configurations as we want, instead of only testing the singleton returned by the module which may become tainted by other tests.

Also you may notice that FakeApp is defined outside of the test constructor, but checkAppMountedOnTarget is not. This is because TestEZ only injects globals like expect into the test constructor, so anything that relies upon those globals must be within. FakeApp operates completely independently of TestEZ so it doesn`t need to be within the test constructor.

local ReplicatedStorage = game:GetService("ReplicatedStorage")

local packages = ReplicatedStorage.Packages
local Roact = require(packages.Roact)

local FAKE_APP_CLASS = "TextLabel"
local FAKE_APP_TEXT = "YEEHAW"

local function FakeApp(_props)
	return Roact.createElement(FAKE_APP_CLASS, {
		Text = FAKE_APP_TEXT;
	})
end

return function()
	local function checkAppMountedOnTarget(target)
		local mountedApp = target:FindFirstChildOfClass(FAKE_APP_CLASS)
		expect(mountedApp).to.be.ok()
		local mountedAppText = mountedApp.Text
		expect(mountedAppText).to.equal(FAKE_APP_TEXT)
	end

	describe("InterfaceService.lua", function()
		local InterfaceServiceModule = require(script.Parent.InterfaceService)
		local serviceConstructor = InterfaceServiceModule.new
		it("should mount interface to target", function()
			local target = Instance.new("Folder")
			local InterfaceService = serviceConstructor()
			InterfaceService._target = target
			InterfaceService._app = FakeApp
			InterfaceService:init()
			checkAppMountedOnTarget(target)
		end)
	end)
end
1 Like
#2

Bloated constructors are kinda annoying, but I am not sure if there is a decent way of avoiding that. It is not like you would be much better off with functional programming in terms of variables for setting up tests.

One upside about the way you wrote the service class is that it looks very maintainable and scalable for both testing and general development.

I think it might be worth it if this is a project that will live for a while.

1 Like
#3

Dependency injection is definitely a way to make testing easier and is common practice.

I would say that what you are doing is not ideal though as you aren’t really injecting dependencies but rather modifying “private” attributes of an object.

E.g.


local InterfaceService = serviceConstructor()
InterfaceService._target = target
InterfaceService._app = FakeApp

It would be more thematic to implement the InterfaceService so you could do this:

local InterfaceService = serviceConstructor(target, FakeApp)
1 Like