@nicoespeon − ConFoo 2023
Slides: bit.ly/build-great-tests
for elevators
in Zhongshan City, China
— source
Freelance Web Dev
Legacy Code specialist 🧟
I organize Meetups 👥
— Software Crafters, TypeScript, and React
Wanna reach out? 👉 @nicoespeon
How can we use that to make tests more readable?
it("returns fetched data on success", async () => {
const data = { some: "data" }
const fetch = async () => ({ data })
const input = await resolveInput(fetch)
expect(input.status).toBe("success")
expect(input.value).toEqual(data)
});
it("returns fetched data on success", async () => {
// Given
const data = { some: "data" }
const fetch = async () => ({ data })
// When
const input = await resolveInput(fetch)
// Then
expect(input.status).toBe("success")
expect(input.value).toEqual(data)
});
It's OK to have multiple asserts
if you test one behavior
it("returns fetched data on success", async () => {
// Given
const data = { some: "data" }
const fetch = async () => ({ data })
// When
const input = await resolveInput(fetch)
// Then
expect(input).toHaveSucceededWith(data)
});
it("returns fetched data on success", async () => {
// Given
const data = { some: "data" }
const fetch = async () => ({ data })
// When
const input = await resolveInput(fetch)
// Then
expectInput(input).toHaveSucceededWith(data)
});
It can be as simple as extracting a function
Don't trust a test you haven't see fail
Let high-level concepts emerge
it("returns the seat that matches the given ID", () => {
const seat = {
id: "1",
type: "seat",
display_name: "A seat",
has_amenities: true,
popularity: 0
};
const bus = {
driver: "left",
cells: [[seat]],
};
const result = getSeatById(bus, "1");
assert.deepEqual(result, seat);
});
it("returns the seat that matches the given ID", () => {
const seat = {
id: "1",
type: "seat",
display_name: "irrelevant",
has_amenities: anyBoolean,
popularity: anyNumber
};
const bus = {
driver: "left",
cells: [[seat]],
};
const result = getSeatById(bus, "1");
assert.deepEqual(result, seat);
});
Don't pick a random value, make it clear
it("returns the seat that matches the given ID", () => {
const seat = {
id: "1",
type: "seat",
display_name: "irrelevant",
has_amenities: anyBoolean,
popularity: anyNumber,
amenities: anyList
};
const bus = {
driver: "left",
floors: [
{
level: 1,
cells: [seat],
},
],
};
const result = getSeatById(bus, "1");
assert.deepEqual(result, seat);
});
it("returns the seat that matches the given ID", () => {
const seat = createAvailableSeat({ id: "1" });
const bus = createBusWithSingleFloor([seat]]);
const result = getSeatById(bus, "1");
assert.deepEqual(result, seat);
});
Only expose what matters
Don't rely on defaults, provide what matters
it("returns the seat that matches the given ID", () => {
const seat = seatFactory()
.withId("1")
.makeAvailable()
.build();
const bus = busFactory()
.withSingleFloorPlan([seat])
.build();
const result = getSeatById(bus, "1");
assert.deepEqual(result, seat);
});
Fluent, flexible API
it("should notify auction was closed when 'close' message is received", () => {
const { listener, translator } = setup();
const message = new Message("SOL Version: 1.1; Event: CLOSE;");
translator.processMessage(null, message);
expect(listener.auctionClosed).toBeCalled();
});
What concept does `null` represent?
it("should notify auction was closed when 'close' message is received", () => {
const { listener, translator } = setup();
const message = new Message("SOL Version: 1.1; Event: CLOSE;");
const UNUSED_CHAT = null;
translator.processMessage(UNUSED_CHAT, message);
expect(listener.auctionClosed).toBeCalled();
});
It's an unused chat. Say it.
const UNUSED_CHAT = null;
it("should notify auction was closed when 'close' message is received", () => {
const { listener, translator } = setup();
const message = new Message("SOL Version: 1.1; Event: CLOSE;");
translator.processMessage(UNUSED_CHAT, message);
expect(listener.auctionClosed).toBeCalled();
});
Can be pulled up, or moved to source code
let listener, translator;
beforeEach(() => {
listener = new FakeAuctionEventListener();
translator = new AuctionMessageTranslator(
"irrelevant sniper ID",
listener,
new FakeFailureReporter()
);
});
// … 120 lines of code
it("should notify auction was closed when 'close' message is received", () => {
const message = new Message("SOL Version: 1.1; Event: CLOSE;");
translator.processMessage(UNUSED_CHAT, message);
expect(listener.auctionClosed).toBeCalled();
});
Don't rely (too much) on before/after hooks
it("should notify auction was closed when 'close' message is received", () => {
const { listener, translator } = setup();
const message = new Message("SOL Version: 1.1; Event: CLOSE;");
translator.processMessage(UNUSED_CHAT, message);
expect(listener.auctionClosed).toBeCalled();
});
It doesn't have to be 1 test per assert
let response, cartId = "irrelevant-cart-id";
before(async () => {
response = await request().get(`/cart/${cartId}`).query({ currency: "EUR" });
});
it("should return 200", () => {
assert.equal(response.status, 200);
});
it("should contain the currency", () => {
assert.equal(response.body.data.currency, "EUR");
});
// … 15 lines of code
it("should have one passenger", () => {
assert.equal(response.body.data.passengers.length, 1);
});
Regroup in one test
Maybe craft a custom assert?
it("should retrieve a one-passenger Cart in the given currency", async () => {
const cartId = "irrelevant-cart-id";
const response = await request()
.get(`/cart/${cartId}`)
.query({ currency: "EUR" });
assert.equal(response.status, 200);
assert.equal(response.body.data.currency, "EUR");
assert.equal(response.body.data.passengers.length, 1);
});
Tests are implicitly coupled…
let response, cartId = "irrelevant-cart-id";
before(async () => {
response = await request().get(`/cart/${cartId}`));
});
// … 30 lines of code
it("should select a seat", async () => {
const query = { passengerId: response.passengers[0].id, seatId: "1" };
const result = await request().put(`/cart/${cartId}/select-seat`).send(query);
assert.equal(result.body.data.selected_seat, "1");
});
Retrieve context data in the "Given" phase…
it("should select a seat", async () => {
const cartId = "irrelevant-cart-id";
const cart = await request().get(`/cart/${cartId}`);
const query = { passengerId: cart.passengers[0].id, seatId: "1" };
const result = await request().put(`/cart/${cartId}/select-seat`).send(query);
assert.equal(result.body.data.selected_seat, "1");
});
… and reduce the noise
it("should select a seat", async () => {
const { cartId, passengers } = await fetchCart();
const seatId = "1";
const result = await request()
.put(`/cart/${cartId}/select-seat`)
.send({ passengerId: passengers[0].id, seatId });
assertSelectedSeatIs(result, seatId);
});
Keep related code together
Reduces Shotgun Surgery
Easier to delete
No need to map 1:1
test & code structure
💡
describe("createBillingAddress", () => {
beforeEach(async () => {
await cleanUpDB();
});
it("creates a BillingAddress with given address and CPF", async () => {
const address = addressBuilder().atStreet("6465 Amazing Street").build();
const billingIds = [createCPF("1234")];
const result = await createBillingAddress(address, billingIds);
assert.equal(result.address1, "6465 Amazing Street");
assert.equal(result.cpf_number, "1234");
assert.equal(await BillingAddressTable.count(), 1);
});
});
Each test is independent
If a test fail, you can inspect the data
A test you can't trust
is worse than no test at all
Usual suspects: Time
Async
Randomness
Coupling
Zero
One
Many
Boundaries
Interface Definition
Exceptions
Simple Scenarios
− source
"To me, legacy code is simply code without tests."
— M. Feathers
See Approval Testing
aka Characterization Tests
aka Snapshot Tests
aka Golden Master
aka Regression Tests
aka Locking Tests…
Pyramid, Trophy, Crab, Honeycomb…
— source
Tests & Software Architecture
3. Most Domain tests, but coarse-grained
3a. Most tests shouldn't prevent refactoring
3b. low-level unit tests to help implement something, can be thrown away
3c. replace actual I/O with fake implementation
4. Need some integrated (contract) tests to remove the blind spots
What about existing code though?
1. Most likely to be your case
2. Identify the I/O, introduce a Seam
2a. Eg. extract method + subclass to test
3. Fine to change code for sake of testing until we are done (safety >> purity)
4. Move to delegate
Cause slowness & flakiness
This is the I/O aka Infrastructure
What's left is Pure Logic aka Domain
1. Separate Infrastructure from Domain
2. Make Domain not depend on Infrastructure
and Functional Core, Imperative Shell… no really, people like to give different names to the same concept… 🤷♂️
Need a workshop on Universal Architecture, Tests, and Legacy Code?
👉 ping me
@nicoespeon to stay in touch
Slides: bit.ly/build-great-tests