3 Practical Tips for Everlasting Tests

3 Practical Tips for Everlasting Tests

In many cases, when I review code or mentor others, I am asked what and how to test a certain functionality. This last week, the debate was hot regarding a new piece of code I was reviewing. So - here's the story and some practical advice on how to write better tests that will change less often and allow you to sleep better.

What Are We Testing?

We have a popup component. This internal component takes care of positioning a popup. While not exposed to consumers, it is used in other more complex components, such as menu, combobox, date picker and more. It can be officially termed as a Facade.

We need to add a new feature to the popup. Here are the specifications:

  1. Add animationFrame property
  2. If animationFrame === true, then, check every frame to see if the popup's anchor element changed size and/or position and run an update. A.K.A. Watcher.
  3. If animationFrame === false, then turn off the watcher.

It was defined in the class like this:

@attr({ mode: 'boolean', attribute: 'animation-frame' }) animationFrame =
		false;

	
animationFrameChanged() {
	this.#updateAutoUpdate();
}

#updateAutoUpdate() {
	this.#cleanup?.();
	if (this.anchorEl && this.open && this.popupEl) {
		this.#cleanup = autoUpdate(
			this.anchorEl,
			this.popupEl,
			() => this.updatePosition(),
			{
				animationFrame: this.animationFrame,
			}
		);
	}
}        

Defining animationFrame is easy. animationFrameChanged calls an update function whenever we update the property. The private method updateAutoUpdate does the following:

  1. It cleans up any popup definitions (#cleanup)
  2. Then, if the popup has an anchor, it is open and the popup element exists, it calls a 3rd party `autoUpdate` method.

Pretty simple. Now, how do we test this feature?

Testing, Mocking, and Coupling

One way to test it would be to mock autoUpdate and make sure it is called with the right parameters:

	describe('animationFrame', () => {
		beforeEach(() => {
			jest.spyOn(floatingUI, 'autoUpdate');
		});

		afterEach(() => {
			jest.mocked(floatingUI.autoUpdate).mockRestore();
		});

		it.each([false, true])(
			'should pass animationFrame=%s as autoUpdate option',
			async (animationFrame) => {
				element.animationFrame = animationFrame;
				await setupPopupToOpenWithAnchor();
				await elementUpdated(element);

				expect(floatingUI.autoUpdate).toHaveBeenCalledWith(
					expect.anything(),
					expect.anything(),
					expect.anything(),
					{ animationFrame }
				);
			}
		);
	});        

In this test, we stub autoUpdate from floatingUI before every test. In the test itself, we change animationFrame's value and expect this value to be sent to autoUpdate.

This is not a bad test in itself. It's really fast to write. But are we losing something when we test this way?

The Price of Mocking

When we mock the 3rd party library this way, we pay a certain price:

  1. The test's description does not match the specifications. From the test itself, we know it is calling a function with some parameter that we just changed - but not much beyond it.
  2. Moreover, the test tells us the internal workings of a private function. Should we write a test for a private function?
  3. When our test includes a 3rd party library in its assert/act part, we implicitly decide to couple ourselves with the 3rd party library.

The clearest outcome of this is - if we decide to replace floatingUI, would we be able to know if our component is still functioning correctly? No, we would not. We would need to change the test again, even though the interface of popup remained the same.

The maybe-less-clear outcome is a test that doesn't follow the test consumer/user flow. The user doesn't care about implementation detail, whether you have a watcher with a third-party library or built it yourself.

Testing Your Interface

The other way to test would be to test your interface as if the 3rd party library does not exist. In other words - test it like a consumer of the component would use it.

Looking back at our specifications:

  1. Add animationFrame property
  2. If animationFrame === true, then, check every frame to see if the popup's anchor element changed size and/or position and run an update. A.K.A. Watcher.
  3. If animationFrame === false, then turn off the watcher.

The first test is easy - we need to make sure we have an animationFrame property with a default value:

it('should be false by default', () => {
	expect(element.animationFrame).toBe(false);
});        

Now we want to test that it doesn't run the watcher when false:

it('should disable recursive calls to requestAnimationFrame when false', async () => {
	await openPopup();
	const cb = getLastFrameCallback();
	rAFStub.mockReset();
	cb();

	expect(rAFStub).toHaveBeenCalledTimes(0);
});
        

And of course, test that it runs the watcher when true:

it('should call rAF recursively when true', async () => {
	element.animationFrame = true;
	await openPopup();
	const cb = getLastFrameCallback();
	rAFStub.mockReset();
	cb();
	cb();
	expect(rAFStub).toHaveBeenCalledTimes(2);
	expect(getLastFrameCallback()).toBe(cb);
});        

One might think we are done - but no. Our spec definitely claims that we should not call the update function if the size or position doesn't change:

it("should prevent call to updatePosition if position or size didn't change", async () => {
	setElementClientRect({ width: 100, top: 100 });
	element.animationFrame = true;
	await openPopup();
	resetMethodCallCount('updatePosition');

	callLastFrameCallback();

	expect(element.updatePosition).toBeCalledTimes(0);
});        

The last things to check are initiating an update when size or position do change:

		it('should updatePosition if size changes', async () => {
			setElementClientRect({ width: 300 });
			element.animationFrame = true;
			await openPopup();
			resetMethodCallCount('updatePosition');
			setElementClientRect({ width: 400 });

			callLastFrameCallback();

			expect(element.updatePosition).toBeCalledTimes(1);
		});

		it('should updatePosition on next frame if position changes', async () => {
			setElementClientRect({ top: 100 });
			element.animationFrame = true;
			await openPopup();
			resetMethodCallCount('updatePosition');
			setElementClientRect({ top: 200 });

			callLastFrameCallback();

			expect(element.updatePosition).toBeCalledTimes(1);
		});        

That's a lot of testing for something we covered with just one mock, right?

And here comes the hard truth - bigger gains take more time and effort.

We gained decoupling (which always sounds good, right?). We are no longer coupled with floatingUI.

In addition, we gained the confidence to make changes - we can remove floatingUI altoghether and we'll know if our replacement did the trick. We don't even need to replace it - just upgrade a version of the library that might change the interface by accident (it's called a bug ;) ). Our component will catch it, and we can do that weekend deployment we always dream of with a safety net.

So... How do I write them better tests? (A.K.A. Summary)

We saw a few principles in this example. I'll start from the most beneficial in this case:

  1. Test your interface - if you expose a property or a method, they should be tested.
  2. Test your interface like a consumer - in your test, build test cases according to the user "stories". In our case, we needed to test the default value - because a user expects the initial value to be false. We also had to test a few more use cases. What happens when we set it to true? What happens when we set true, and the size or position remains the same? What happens when they change? I didn't invent these stories - they are part of the specification we agreed upon when we defined our interface.
  3. Use Facades to hide third-party libraries and expose a simpler, unified interface. You can then use the Facade across your app/module, and if you wish to replace it, it's a single-place replacement. You'd also expect its tested interface to be used instead of testing everything repeatedly.

What do you think? Do these benefits worth the effort?



Guillaume Faas

Senior .Net Developer Advocate | Speaker | Coach | Mentor

5 个月

The "price to pay" is an investment, which will allow you to go faster in the future. It's totally worth it IMO.

要查看或添加评论,请登录

社区洞察

其他会员也浏览了