POC for polysemy

[newhoggy]

Changelog

- description: |
    POC for polysemy
# uncomment types applicable to the change:
  type:
  # - feature        # introduces a new feature
  # - breaking       # the API has changed in a breaking way
  - compatible     # the API has changed but is non-breaking
  # - optimisation   # measurable performance improvements
  - refactoring    # QoL changes
  # - bugfix         # fixes a defect
  # - test           # fixes/modifies tests
  # - maintenance    # not directly related to the code
  # - release        # related to a new release preparation
  # - documentation  # change in code docs, haddocks...

Context

Using polysemy to simplify tests.

How to trust this PR

Highlight important bits of the PR that will make the review faster. If there are commands the reviewer can run to observe the new behavior, describe them.

Checklist

• [ ] Commit sequence broadly makes sense and commits have useful messages
• [ ] New tests are added if needed and existing tests are updated. See Running tests for more details
• [ ] Self-reviewed the diff

[smelc]

In the current state, I am neutral:

1. We don't really save on boilerplate
2. We don't add boilerplate either
3. The multiple reader context is nice to have, but not essential.
4. If we introduce a dependency to https://github.com/tek/polysemy-conc (unclear if we do really), it's not great; because this is a library with very few users (even if, I must say, I am the manger of the library's author @tek :rofl: who works at Modus Create)

[newhoggy]

One advantage is that you could log in the functions being tested and then capture those logs in a test annotation like this:

           ┏━━ test/cardano-cli-golden/Test/Golden/Byron/SigningKeys.hs ━━━
        38 ┃ hprop_deserialise_nonLegacy_signing_Key :: Property
        39 ┃ hprop_deserialise_nonLegacy_signing_Key = propertyOnce $ localWorkspace $ do
        40 ┃   skeyBs <- LBS.readFile "test/cardano-cli-golden/files/input/byron/keys/byron.skey1"
           ┃   │ [info]  [T.G.B.SigningKeys#40] Reading bytestring from file: test/cardano-cli-golden/files/input/byron/keys/byron.skey1
        41 ┃     & trapFail @IOException
           ┃     ^^^^^^^^^^^^^^^^^^^^^^^
           ┃     │ Expected Right: test/cardano-cli-golden/files/input/byron/keys/byron.skey1: openBinaryFile: does not exist (No such file or directory)
        42 ┃ 
        43 ┃   fromEither (deserialiseFromBytes Crypto.fromCBORXPrv skeyBs)
        44 ┃     & trapFail @DeserialiseFailure
        45 ┃     & void

The line:

           ┃   │ [info]  [T.G.B.SigningKeys#40] Reading bytestring from file: test/cardano-cli-golden/files/input/byron/keys/byron.skey1

Is logging from the readFile function, which is pretty short but we could have logging in a more elaborate function and when we get a failure that logging could give some insight into what the function we are testing isn't behaving as expected.

[newhoggy]

so I'm wondering why polysemy and not some other effect system?

Mainly due to familiarity and ecosystem - in particular the logging library polysemy-log. We could select a different effect system but we may need to implement the libraries we want and we should also apart from performance evaluate how good the errors are.

[smelc]

All in all, I'm not convinced:

1. There is no significant reduction of boilerplate
2. By adding dependencies to https://github.com/tek/polysemy-conc and https://github.com/haskell-works/hw-polysemy we put ourselves in a very niche part of the Haskell ecosystem, and this is not good for long term maintenance. The bus factor here would be very small.

If we are convinced this is important, I would wait a few months to see if this part of the ecosystem gains traction.

[github-actions[bot]]

This PR is stale because it has been open 45 days with no activity.