Josh Steadmon <steadmon@xxxxxxxxxx> wrote: > On 2024.07.02 22:59, Ghanshyam Thakkar wrote: > > Josh Steadmon <steadmon@xxxxxxxxxx> wrote: > > > I think this commit in particular shows how TEST_RUN() is more > > > convenient than TEST(). (Although, arguably we shouldn't have allowed > > > the setup() + callback situation to start with.) > > > > Could you expand a bit on why the setup() + callback thing shouldn't be > > allowed? I think it is a nice way of avoiding boilerplate and having > > independent state. > > It's a matter of taste rather than strict principles, I suppose, but I > think that test cases should focus on being readable rather than > avoiding duplication. Production code can follow a Don't Repeat Yourself > philosophy, but when a test breaks, it's better to be able to look at > only the failing test and quickly see what's wrong, rather than having > to work out which functions call which callbacks and what gets verified > where. Also, since we don't have tests for our tests, it's important > that the tests are easy for people to manually verify them. Makes sense. But the manner in which callbacks are used in these tests, which usually only initialize a certain datastructure and then release them at the end, I fail to see them as more harder to manually verify. But it is very easy to stretch the line and perhaps do more than initialization in the setup(), so I can see how disallowing them would get rid of the need of judging every time wether they are readable or not. Maybe a 'best practices' document would help in that regard? > I also agree with René's point about the test cases matching real-world > API usage. > > I do agree that having separate functions improves isolation, but > hopefully we can encourage test authors to define local variables in > their TEST_RUN blocks. Makes sense. Thanks.
