nitpick: I'd either move the '}' to the above line or move the return statement down to its own line. Same comment for the next test.

It's weird that grails and gorm doesn't really provide a method of stubbing domain methods. This feels weird to have to do but I couldn't find a better solution from googling.

nitpick: I'd either move the '}' to the above line or move the return statement down to its own line. Same comment for the next test.

I did it like that before and the run autoformatting in Intellij 😄

It's weird that grails and gorm doesn't really provide a method of stubbing domain methods.

There are. We just need to do some more work on testing to start using them. I'm hoping a test-a-thon exercise will help us uncover more testing best practices.

why Spy? Does it not still work if you make it a Stub? From what I see in the docs, we should generally avoid using Spy if at all possible. https://grails.org/blog/2018-06-22.html

This line:

1 * inventoryService.generateTransactionNumber()

doesn't work when I change it to Stub, but I think this one check doesn't make too much sense

oh I thought you could still do that with Stub. My bad.

But yes maybe we don't need to care if that internal method call is being made. IMO tests should only care about the inputs and return values of the method that they're testing. The internals should be a black box (which also makes refactoring easier since we're not making assumptions about the internal code, which may change).

kinda weird but I can't think of a better solution off the top of my head outside of just asserting on each key of the map.

what about quantityCancelled null and 0?

Comparing null with value doesn't throw an exception, I can add this case in the test above

shouldn't quantity for these lines be 3? Otherwise they wouldn't be cancelled regardless.

or I guess a better question is do we also want to add lines where quantity is 3 for each of these combos and assert that isCanceled is true

When you have simple one line asserts like this you can do:

expect:
domain.quantityNotCanceled() == quantityNotCanceled

probably also worth having a line where quantityCancelled is > 0 but isSubstituted is still false if there are no substitutionItem

could be nice to add some test cases around the modificationItem being set since I see that's used in the isCanceledDuringPick() method. Maybe something similar to what you did for the last test you have in this file. Up to you though.