Closed auduchinok closed 4 years ago
@auduchinok Yes indeed. It's probably just an oversight.
Change logger.logWithError (...)
to config.printers.info (...)
on this line: https://github.com/haf/expecto/blob/master/Expecto/Expecto.fs#L1953 — that way tooling can inject its own loggers.
This is a perfect getting-started issue!
Are you sure about exn
? It takes both a string and an exception which means we'd have to create some artificial exception probably duplicating the string. It also takes a time span which we don't have, though it could be just 0. info
seems to suit better given parameter types it expects.
If sticking with exn
, should a separate exception class be added or TestDiscoveryException
is OK?
Sure, info
— nice catch.
Could it be better for tooling if the error about duplicate names was reported via relevant printer instead of being written to the output?
exn
andinfo
printers seem to suite well enough.Although it would probably be a better design, the tooling would have to still support current approach as some existing projects may not get an updated Expecto package anytime soon. Due to this I'm not sure if it'd be a better overall situation for the tools but just wanted to share the thoughts.