On Sat, May 8, 2021 at 4:05 AM 'Brendan Higgins' via KUnit Development <kunit-dev@xxxxxxxxxxxxxxxx> wrote: > > On Thu, May 6, 2021 at 10:09 PM David Gow <davidgow@xxxxxxxxxx> wrote: > > > > The use of typecheck() in KUNIT_EXPECT_EQ() and friends is causing more > > problems than I think it's worth. Things like enums need to have their > > values explicitly cast, and literals all need to be very precisely typed > > for the code to compile. > > nit: I have not had the typecheck() call prevent any code from > compiling, just generating warnings. I guess you can have a build set > to cause any warning to be promoted to an error; still, I think this > statement is misleading. > Whoops -- it was the issue in patch 2 that was causing the error. This is indeed just a warning. I'll send out a second version with a more accurate description next week, assuming no further issues appear. > > While typechecking does have its uses, the additional overhead of having > > lots of needless casts -- combined with the awkward error messages which > > don't mention which types are involved -- makes tests less readable and > > more difficult to write. > > > > By removing the typecheck() call, the two arguments still need to be of > > compatible types, but don't need to be of exactly the same time, which > > seems a less confusing and more useful compromise. > > > > Signed-off-by: David Gow <davidgow@xxxxxxxxxx> > > Looks good to me. > > Reviewed-by: Brendan Higgins <brendanhiggins@xxxxxxxxxx> > > > --- > > > > I appreciate that this is probably a bit controversial (and, indeed, I > > was a bit hesitant about sending it out myself), but after sitting on it > > for a few days, I still think this is probably an improvement overall. > > > > The second patch does fix what I think is an actual bug, though, so even > > if this isn't determined to be a good idea, it (or some equivalent) > > should probably go through. > > I don't remember being a huge fan of the typecheck when it was asked > for either. I think I am a little bit more indifferent than you; > nevertheless, I support this change. > > -- > You received this message because you are subscribed to the Google Groups "KUnit Development" group. > To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@xxxxxxxxxxxxxxxx. > To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/CAFd5g44bot7S-Ya7s7QxnKfXHcy8WxUcNPsZuw_qWMaAQbqLCg%40mail.gmail.com.
