Ava: Setup Functions (t.teardown)

Created on 18 Jul 2018 · 13Comments · Source: avajs/ava

This is going to be a bit of a more testing-philosophy proposal. I'm so sorry for writing such a long issue, but it's mostly code examples of the same test suite so hopefully it won't take very long to get through.

TL;DR

It's very hard to write setup and teardown (beforeEach/afterEach) functions _"properly"_ in pretty much every test framework that exists today. To solve this, what if AVA had "Setup Functions":

function createComponent(t, opts) {
  let component = new Component(opts)
  t.teardown(() => component.destroy())
  return component
}

test("component when foo does this thing", t => {
  let component = createComponent(t, { option: true })
  // ...
})

The Problem: Setting stuff up for your tests

BDD-style

For a while now, the JavaScript community has been writing "BDD-style" tests like:

describe("Component", () => {
  let component

  beforeEach(() => { 
    component = new Component()
  })

  describe("when foo", () => {
    let foo;

    beforeEach(() => { 
      foo = new Foo()
    })

    it("should do this thing", () => {
      expect(component.doThisThing(foo)).toBe(true)
    })
  })

  describe("when bar", () => {
    let bar

    beforeEach(() => { 
      bar = new Bar()
    })

    it("should do this thing", () => {
      expect(component.doThisThing(bar)).toBe(true)
    })

    describe("and baz", () => {
      let baz

      beforeEach(() => { 
        baz = new Baz()
      })

      it("should do this thing", () => {
        expect(component.doThisThing(bar, baz)).toBe(true)
      })
    })
  })
})

This has some notable problems:

Lots of refactoring when:
- You need to add tests that use value that are setup in other describe()'s (i.e. a test that uses both Foo and Baz would need to hoist setup code or duplicate it)
- You need to configure objects setup in beforeEach() differently in individual tests (i.e. passing a different option to new Bar() deep within bar + baz tests)

AVA-style

In contrast, this is one way you could write that same test suite in AVA:

import test from "ava"

test.beforeEach(t => { 
  t.context.component = new Component()
  t.context.foo = new Foo()
  t.context.bar = new Bar()
})

test("component when foo does this thing", t => {
  t.true(t.context.component.doThisThing(foo))
})

test("component when bar does this thing", t => {
  t.true(t.context.component.doThisThing(t.context.bar))
})

test("component when bar and baz does this thing", t => {
  t.true(t.context.component.doThisThing(t.context.bar, t.context.baz))
})

There are still problems here though:

You are setting up more than you need to for every test
You can't configure how things get setup at all.

AVA+setup functions-style

So instead, you'd probably write it like this:

let createComponent = () => new Component()
let createFoo = () => new Foo()
let createBar = () => new Bar()

test("component when foo does this thing", t => {
  let component = createComponent()
  let foo = createFoo()
  t.true(component.doThisThing(foo))
})

test("component when bar does this thing", t => {
  let component = createComponent()
  let bar = createBar()
  t.true(component.doThisThing(bar))
})

test("component when bar and baz does this thing", t => {
  let component = createComponent()
  let bar = createBar()
  let baz = createBar()
  t.true(component.doThisThing(bar, baz))
})

This works great because:

Your tests and setup functions are extremely portable
Things are only setup when you need them
Your tests write out exactly what they require
You can provide options to your setup functions

Tearing setup functions down

However, there's still a problem:

What happens when you do some cleanup of the things you created?

With test.beforeEach this is easy:

test.always.afterEach(t => {
  t.context.component.destroy()
  t.context.foo.destroy()
  t.context.bar.destroy()
})

But with setup functions, it gets trickier. You end up writing something like this:

let _component
let _foo
let _bar

function createComponent() {
  let _component = new Component()
  return _component
}

function createFoo() {
  let _foo = new Foo()
  return _foo
}

function createBar() {
  let _bar = new Bar()
  return _bar
}

test.always.afterEach(() => {
  _component.destroy()
  _foo.destroy()
  _bar.destroy()
})

// ...

Tearing setup functions down - Part 2

Except... that doesn't work:

_foo and _bar are not always created so you have to check if they exist first
tests are (generally) run concurrently, so these values get re-assigned and you teardown some instances multiple times and others not at all

Instead you need to write it like:

function createComponent(t) {
  t.context.component = new Component()
  return t.context.component
}

function createFoo() {
  t.context.foo = new Component()
  return t.context.foo
}

function createBar() {
  t.context.bar = new Component()
  return t.context.foo
}

test.always.afterEach(t => {
  if (t.context.component) t.context.component.destroy()
  if (t.context.foo) t.context.foo.destroy()
  if (t.context.bar) t.context.bar.destroy()
})

test("component when foo does this thing", t => {
  let component = createComponent(t)
  let foo = createFoo(t)
  t.true(component.doThisThing(foo))
})

// ...

This is a lot to have to remember, so I want to propose an alternative:

The (Proposed) Solution: `t.teardown()`

What if we instead tried placing the teardown code within the setup functions?

import test from "ava"

function createComponent(t) {
  let component = new Component()
  t.teardown(() => component.destroy())
  return component
}

function createFoo(t) {
  let foo = new Foo()
  t.teardown(() => foo.destroy())
  return foo
}

function createBar(t) {
  let bar = new Bar()
  t.teardown(() => bar.destroy())
  return bar
}

test("component when foo does this thing", t => {
  let component = createComponent(t)
  let foo = createFoo(t)
  t.true(component.doThisThing(foo))
})

test("component when bar does this thing", t => {
  let component = createComponent(t)
  let bar = createBar(t)
  t.true(component.doThisThing(bar))
})

test("component when bar and baz does this thing", t => {
  let component = createComponent(t)
  let bar = createBar(t)
  let baz = createBar(t)
  t.true(component.doThisThing(bar, baz))
})

t.teardown(cb) would:

Schedule cb() to be run after the currently running test (which it knows because it's based on TestContext)
Always run regardless if the test passed or failed

This solves the entire problem space:

Setup code is only run when needed
Tests write out exactly what setup code they need
Tests and setup code is extremely portable
Setup code can be configured by the tests
Setup code gets torn down correctly with async tests
Teardown code only runs when needed

I would like to see AVA implement something like this and promote it within the docs.

Open Questions

If t.teardown() throws an error, should it fail the current test or report a separate failure?

IssueHunt Summary

ulken has been rewarded.

Backers (Total: $80.00)

issuehunt ($80.00)

Submitted pull Requests

- #2459 Add `t.teardown()`

Tips

Checkout the Issuehunt explorer to discover more funded issues.
Need some help from other developers? Add your repositories on IssueHunt to raise funds.

Rewarded on Issuehunt enhancement help wanted test-interface

Source

jamiebuilds

❤32 👍9 🎉1

All 13 comments

That's a neat idea @jamiebuilds.

This could be done using a try {} finally {} block inside the test implementation, but that's not very pretty. I'd be OK with adding a teardown util.

I think the callbacks should execute within the test, but orchestrated by AVA. They have access to the t object through the closure so they could still perform assertions.

Presumably callbacks are executed in order, serially, waiting for any returned promises to settle? Errors wouldn't stop the next callback from executing?

novemberborn on 19 Jul 2018

If an error gets thrown during teardown it’s likely you want to fail the test. Teardown is theoretically cleaning up side-effects to put the suite back in to a known state. an error during cleanup is a warning that subsequent tests will run in an unknown state. if you ignore those, subsequent test failures may be cryptic.

chrisdarroch on 19 Jul 2018

👍1

Presumably callbacks are executed in order, serially, waiting for any returned promises to settle? Errors wouldn't stop the next callback from executing?

So I have two opposing thoughts:

I think it would be good to mimic the semantics of before/after(Each) as much as possible.
I think teardown code shouldn't care about being run in a set order and should be able to run concurrently.

if you ignore those, subsequent test failures may be cryptic.

I don't think you have to ignore them, I think it would be possible to report an error in teardown separately from the test itself passing. The test itself could also be failing and you'd still want the teardown code to run, which could then also fail (likely for the same reason as the test failure), and you'd want both errors to be reported.

That being said I can see arguments for both choices.

jamiebuilds on 19 Jul 2018

👍1

The before & after hooks run concurrently by default, are invoked in order, and can be made to run serially.

I don't want to add t.teardown.serial(), so the question is what the default behavior should be. I propose we make them execute in order and serially. You can always compose different behavior in an initial teardown callback, if needed for performance reasons.

Before & after hooks can execute their own assertions, as they receive their own t value. I reckon the callback will be a closure with access to the t value of the test. We could make it crash if it performs an assertion on that object, but that seems like a pitfall. Providing its own t value just leads to shadowing:

test('test', t => {
  t.teardown(t => {
    // Which value is `t`?
  })
})

That doesn't seem ideal either. However if the teardown can perform assertions on the test, then I don't think we should report teardown failures separately.

Tests keep executing even if there is an assertion failure. We can do the same for teardown callbacks. There's an open issue for AVA to report multiple assertion failures though.

novemberborn on 21 Jul 2018

I propose we make them execute in order and serially

Okay, that sounds good 👍

test('test', t => {
  t.teardown(t => {
    // Which value is `t`?
  })
})

It seems a bit unnecessary to provide t.teardown's callback with t. Any scope where you can call t.teardown you should already have access to t.

I also want this to work without any weirdness:

t.teardown(db.close);
// Does db.close receive an unexpected `t` argument? What if that changed its behavior

However if the teardown can perform assertions on the test, then I don't think we should report teardown failures separately.

That's fair 👍

jamiebuilds on 23 Jul 2018

@jamiebuilds are you still interested in implementing this? 😄

havenchyk on 10 Oct 2018

I had started on an implementation somewhere but if you're wanting to grab it feel free

jamiebuilds on 10 Oct 2018

No-no, it will take ages for me to implement this @jamiebuilds

I just found this idea very cool and useful and I wanted to check was there any activity after the last comment at this issue

havenchyk on 10 Oct 2018

While not exactly the same, I noticed in the 1.0 release docs that you can now do helper setup functions. https://github.com/avajs/ava/blob/master/docs/recipes/puppeteer.md

btkostner on 16 Dec 2018

@issuehunt has funded $80.00 to this issue.

Submit pull request via IssueHunt to receive this reward.
Want to contribute? Chip in to this issue via IssueHunt.
Checkout the IssueHunt Issue Explorer to see more funded issues.
Need help from developers? Add your repository on IssueHunt to raise funds.

IssueHuntBot on 10 May 2019

Can't this be solved by using helper functions and variations?

async function withComponent(t, run) {
  const component = {
    name: 'Dropdown',
    alive: true,
    destroy() {
      this.alive = false
    }
  }

  await run(t, component)
  component.destroy();
}

test("component", withComponent, async (t, comp) => {
  t.is(comp.name, 'Dropdown')
  t.true(comp.alive)
})

Unfortunately this feature has not be documented well, other than a mention here.

tryzniak on 23 Aug 2019

Yes but it's hard to get that right. Like, you need a try / finally to ensure the component is destroyed if run() throws. But you probably don't want the error from component.destroy() to replace the one from run()… So a teardown utility may still be useful.

novemberborn on 24 Aug 2019

@novemberborn has rewarded $72.00 to @ulken. See it on IssueHunt