UnkindPartition / tasty

Modern and extensible testing framework for Haskell
638 stars 108 forks source link

Do not depend on clock package, decomission clock flag #345

Closed Bodigrim closed 2 years ago

Bodigrim commented 2 years ago

Starting from GHC 8.4 we can use GHC.Clock from base, and for diminishing minority of users of GHC < 8.4 time is sufficiently good (and does not require build, because it is a boot package).

andreasabel commented 2 years ago

for diminishing minority of users of GHC < 8.4 time is sufficiently good

I take your word for it...

Bodigrim commented 2 years ago

@andreasabel the difference between clock and time is observable only if a user changes system clock while running tests. System time change can lead to spurious timeouts, if time is used. But I seriously doubt plausibility of such scenario: nowadays builds with GHC < 8.4 are mostly happening on CI, not on user machines. And any timeout can fail depending on current system activity anyway.

Note that clock flag, switching between clock and time, was introduced for GHCJS, which does not support FFI calls required for clock.

ncfavier commented 1 year ago

Reintroduces https://github.com/UnkindPartition/tasty/issues/288 in a different form: ReferenceError: h$getMonotonicNSec is not defined. This was apparently known so I'm not sure why this was merged? Is GHCJS no longer supported?

Bodigrim commented 1 year ago

What exactly was “apparently known”?

No CI job = not supported. If you want us to consider GHCJS support, please contribute one.

ncfavier commented 1 year ago

GHCJS, which does not support FFI calls required for clock.

I guess you didn't know that GHCJS also doesn't support GHC.Clock. That settles my confusion.

andreasabel commented 1 year ago

If there is a macro akin to __GLASGOW_HASKELL__ in GHCJS, once could add it to the conditions of import GHC.Clock. https://github.com/UnkindPartition/tasty/blob/9ff66c2339ba72711185111afe9eddbf3ce5e0fc/core/Test/Tasty/Runners/Utils.hs#L15-L19

ncfavier commented 1 year ago

There is ghcjs_HOST_OS, but I think that would also affect the new GHC 9.6 JS backend (which is not to be confused with GHCJS, although it uses ghcjs as its OS name...), which seems to support GHC.Clock.

Anyway, since GHCJS is on the way out anyway I've just patched this in nixpkgs.