Gracefully handling getters/setters for Jest spies

January 22, 2022

#Introduction

Last week at my job we were some writing some new tests for a utility function that depends on calling window.navigator.platform. navigator.platform isn’t a normal function — it’s a getter — and the function that uses it uses it internally, which means we need to spy on it. During the creation of these tests some nice discussion about not only how to spy on it, but how to spy on it nicely, came up and sparked some interesting thoughts and ideas that I’d like to present here.

#Spies

Spies in Jest are great. They’re very straightforward and don’t really require much thought:

it('should open a new window when the button is clicked', () => {
  // Create a spy on `window.open` with a mock return value
  const windowOpenSpy = jest.spyOn(window, 'open').mockReturnValue(true);

  render(<OpenExternalSetup />);

  click(screen.getByRole('open external window'))

  expect(windowOpenSpy).toBeCalledWith('https://example.com/special-token');
});

While this works perfectly fine, that new windowOpenSpy variable is unnecessary, since window.open was overridden during the spy setup. We can simplfy it:

it('should open a new window when the button is clicked', () => {
  jest.spyOn(window, 'open').mockReturnValue(true);

  render(<OpenExternalSetup />);

  click(screen.getByRole('open external window'))

  expect(window.open).toBeCalledWith('https://example.com/special-token');
});

Comparing the two, I very much prefer the latter. It eliminates the use of a variable, which is more “graceful”, and it arguably reads a little easier too. Sure, windowOpenSpy is easy to reason about, and it does have the benefit of making it abundantly clear that we’re dealing with a spy, but we already know we’re dealing with a spy when using expect and not needing to know and think about that variable reduces what we need to keep in our minds.

#Spying on getters

Avoiding that variable becomes a bit trickier when dealing with getters though (setters too, and when I refer to getters in this post, I mean both). If we only need to overwrite a getter or need to mock its implementation once, the test is pretty straightforward like above:

it('should return linux if the platform has a prefix of linux', () => {
  jest.spyOn(window.navigator, 'platform', 'get').mockReturnValue('linux-something');

  expect(utils.getPlatform()).toBe('linux');
});

In reality we rarely ever need to spy on a function once, since the code that’s using it almost certainly has branches and therefore will require multiple test cases, and so we need a way to handle spying across multiple test cases. The most straightforward way would be to just spy in each test case, but that would add a lot of repetition, so how can we reduce that? We know that jest.spyOn returns the spied function, so we can define a variable inside of the describe block and simply overwrite it each time. Let’s try that:

describe('utils.getPlatform', () => {
  let navigatorPlatformGetterSpy;

  beforeEach(() => {
    // We need to do this in `beforeEach` since we've enabled `restoreMocks` in our config
    navigatorPlatformGetterSpy = jest.spyOn(window.navigator, 'platform', 'get');
  });

  it('should return mac if the platform has a prefix of mac', () => {
    navigatorPlatformGetterSpy.mockReturnValue('mac-something');
    expect(utils.getPlatform()).toBe('mac');
  });

  it('should return linux if the platform has a prefix of linux', () => {
    navigatorPlatformGetterSpy.mockReturnValue('linux-something');
    expect(utils.getPlatform()).toBe('linux');
  });
});

Perfectly reasonable. But what if we wanted to eliminate that variable? How could we approach it?

describe('utils.getPlatform', () => {
  beforeEach(() => {
    jest.spyOn(window.navigator, 'platform', 'get');
  });

  it('should return mac if the platform has a prefix of mac', () => {
    /*
Hmm... we can't access the getter doing something like `window.navigator.platform.get`...
How can we access it? Is there a symbol, like `window.navigator.platform[GetterSymbol]`?
*/ expect(utils.getPlatform()).toBe('linux'); }); it('should return linux if the platform has a prefix of linux', () => { expect(utils.getPlatform()).toBe('linux'); });
});

Unfortunately the answer to that second question is no, there is no symbol. But there is a function that we can use called Object.getOwnPropertyDescriptors:

const navigatorPlatformGetter = Object.getOwnPropertyDescriptors(window.navigator).platform.get;

// We can also get it similarly to the way `jest.spyOn` works
const navigatorPlatformGetter = Object.getOwnPropertyDescriptor(window.navigator, 'platform').get;

Now how would it look if we used getOwnPropertyDescriptor?

describe('utils.getPlatform', () => {
  beforeEach(() => {
    jest.spyOn(window.navigator, 'platform', 'get');
  });

  it('should return mac if the platform has a prefix of mac', () => {
    Object.getOwnPropertyDescriptor(window.navigator, 'platform').get.mockReturnValue('mac-something');
    expect(utils.getPlatform()).toBe('mac');
  });
});

A bit verbose. Let’s pull that out to a nicer-to-use function:

function getter(object, property) { return Object.getOwnPropertyDescriptor(object, property).get;
}

describe('utils.getPlatform', () => { beforeEach(() => { jest.spyOn(window.navigator, 'platform', 'get'); }); it('should return mac if the platform has a prefix of mac', () => { getter(window.navigator, 'platform').mockReturnValue('mac-something'); expect(utils.getPlatform()).toBe('mac'); });
});

Yes! We’ve eliminated that variable, and it reads pretty fluidly too: “the getter of window.navigator.platform has a mocked return value of ‘mac-something’”. To me this feels like a win.

Is it really worth it though, when the former was more than adequate and the biggest issue with that approach — possible poor names for the spy variable — could be caught during code review? Maybe not, and the mutable variable approach does feel natural in JavaScript.

However, I’m not using JavaScript at work — I’m using ClojureScript, sorry for burying the lede — and this changes things a bit.

#How about ClojureScript?

At Pitch we’ve setup Jest to work in ClojureScript and it looks and works quite similarly to the vanilla JavaScript version:

(it "should open a new window when the button is clicked"
  (spy-on js/window :open true)
  (render [open-external-setup])

  ; get-by implicitly works on screen
  (click (get-by :role "open external window"))
  (called-with? js/window.open "https://example.com/special-token"))

How about the getters? Well, just like in the example above, we can use a mutable variable, an atom in Clojure:

(describe "utils/get-platform"
  (let [!nav-platform-getter-spy (atom nil)]
    (before-each (reset! !nav-platform-getter-spy (spy-on js/window.navigator :platform :get)))

    (it "should return :mac if the platform has a prefix of mac"
      (.mockReturnValue @!nav-platform-getter-spy "mac-something")
      (=? :mac (utils/get-platform)))

    (it "should return :linux if the platform has a prefix of linux"
      (.mockReturnValue @!nav-platform-getter-spy "linux-something")
      (=? :linux (utils/get-platform)))))

While this works, it doesn’t feel clean since I’m using a mutable variable, and in general I really try to avoid having a mutable variable unless it’s not possible otherwise. We know that it is possible to access the getter using a function instead, and in that context I think the question of is it worth it? does change a little bit in favor of the function:

(defn getter "Return the getter function, if it exists, for `property` on `object`." [object property] (let [getter-fn (.-get (js/Object.getOwnPropertyDescriptor object property))] (if (= js/undefined getter-fn) nil getter-fn)))

(describe "utils/get-platform" (before-each (spy-on js/window.navigator :platform :get)) (it "should return :mac if the platform has a prefix of mac" (.mockReturnValue (getter js/window.navigator :platform) "mac-something") (=? :mac (utils/get-platform))) (it "should return :linux if the platform has a prefix of linux" (.mockReturnValue (getter js/window.navigator :platform) "linux-something") (=? :linux (utils/get-platform))))

It’s still arguable if this is better than just using an atom, and it’s definitely more complex so there is an argument to be made against it for that reason alone, but I personally don’t mind a bit of complexity for cleaner code, especially if it helps with readability and reducing mutability.

What do you think? Is it worth it? What would you go with in practice: just using the mutable variable, or would you opt for that function? ◾️