Please critique this TDD flow

#36090

I have a domain entity called a "Booking Window" that has a Start Date and End Date, and defines a period in which a Member can make a Booking.? The business is a mobile car wash, and Members can book a wash every 14 days.

I have a means of establishing the first Booking Window - at the point of the Member signing up.? Now I feel like the next most useful function is one that finds all Booking Windows closing "today" and makes new ones.

So I wrote a test like this (kinda pseudo coded)

test("nothing to do when no booking windows at all", async () => {

? ? const client = stubGraphQlClientContainingNothing()

? ? const clock = new FixedClock(new Date())

? ? const windows = await makeTodaysBookingWindows(client, clock)

? ? expect(windows).toHaveLength(0)

})

and my function was just

export async function makeTodaysBookingWindows(client: GraphQlClient, clock: Clock): Promise<BookingWindow[]> {

? ? return []

?

}

Then I decided I needed a test of the case that had bookings, but none ending today:

test("nothing to do when no booking windows end today", async () => {

? ? const client = stubGraphQlClientContaining(bookingWindowEnding(tomorrow()))

? ? const clock = new FixedClock(new Date())

? ? const windows = await makeTodaysBookingWindows(client, clock)

? ? expect(windows).toHaveLength(0)

?

})

and the same function implementation allowed this to pass.

Now I decided I need to do a real case, of one Booking Window ending today:

test("create a new booking window when one expires today", async () => {

? ? const client = stubGraphQlClientContaining(bookingWindowEnding(today()))

? ? const clock = new FixedClock(new Date())

? ? const windows = await makeTodaysBookingWindows(client, clock)

? ? expect(windows).toHaveLength(1)

? ? expect(window[0].startDate).toBe(today())

? ? expect(window[0].endDate).toBe(today() + 14 days)

?

})

and that forced me to make some production code.

Now I am left thinking, what next?? The code I wrote to make the above test pass does not loop, so I need to cover the case where there are many Booking Windows ending today, so I write this:

test("create a new booking window for every window ending today", async () => {

? ? const client = stubGraphQlClientContaining(bookingWindowEnding(today()), bookingWindowEnding(today()))

? ? const clock = new FixedClock(new Date())

? ? const windows = await makeTodaysBookingWindows(client, clock)

? ? expect(windows).toHaveLength(2)

? ? expect(window[0].startDate).toBe(today())

? ? expect(window[0].endDate).toBe(today() + 14 days)

? ? expect(window[1].startDate).toBe(today())

? ? expect(window[1].endDate).toBe(today() + 14 days)

?

})

Now I probably have the production function I need, but the "create a new booking window when one expires today" test case is now irrelevant, so I guess the right thing to do is remove it.

Anyway, thats enough to communicate my approach to this function.? It doesn't "feel" great, so I'd appreciate knowing how others would approach this.

Thanks!

#36091

Hi Mike,

?

my first impression is that you have some interesting logic that is now coupled to your GraphQL engine.? Are you sure you want this?? Since the business logic around your use case might be non-trivial, then implementing it all in GraphQL could be non-trivial and the resulting code might by hard to change.? It's all fair if the logic is very stable and you need every drop of performance.? An alternative way would be to query GraphQL for the minimal set of data that your logic needs to make a decision, and then implement the interesting business logic in a separate function, that is pure and synchronous.

?

I also question whether you really want a single function that does querying, business logic, and saving an updated state on the DB.? Perhaps you would be better served by 3 separate functions.

?

Hope this?helps a bit,

?

Matteo

#36092

On Thu, Jul 7, 2022 at 10:16 AM Matteo Vaccari <matteo.vaccari@...> wrote:

Hi Mike,
?

my first impression is that you have some interesting logic that is now coupled to your GraphQL engine.? Are you sure you want this?? Since the business logic around your use case might be non-trivial, then implementing it all in GraphQL could be non-trivial and the resulting code might by hard to change.? It's all fair if the logic is very stable and you need every drop of performance.? An alternative way would be to query GraphQL for the minimal set of data that your logic needs to make a decision, and then implement the interesting business logic in a separate function, that is pure and synchronous.

I have a similar impression. You will probably notice, if you continue this way, that you will eventually be duplicating a lot of silly details about GraphQL in your tests, even though the code you write ends up being mostly plain JavaScript to make the new tests pass. At some point, you might decide that this feels wrong somehow and it would help you to isolate the plain JavaScript behavior from the GraphQL parts. This would make the tests simpler and execute more quickly, which improves your feedback loop and helps you change the "core business" behavior more easily.

I also question whether you really want a single function that does querying, business logic, and saving an updated state on the DB.? Perhaps you would be better served by 3 separate functions.

Again, I agree that this will probably eventually happen. The same advice applies: if you continue by putting everything in one place, then eventually you'll notice that some of your tests have copy/paste duplication of distracting details, and if you separate these three bits of behavior into separate pieces, then the distracting details go away and the remaining details feel much more relevant and meaningful. I interpret this as a good sign, generally.

Good luck.

--

J. B. (Joe) Rainsberger :: ?:: ::

--
J. B. (Joe) Rainsberger :: :: ::
Teaching evolutionary design and TDD since 2002

开云体育

Please critique this TDD flow