Integration with CLB emulator and bet-ref example #1655
Conversation
Release v9.3.0
Release v9.3.1
euonymos
changed the title
euonymos
force-pushed
the
euonymos/clb
branch
from
1ea10e1 to
e78752c
Compare
There was a problem hiding this comment.
Nice PR!
I would not merge it into develop just yet until the issues listed in the changelog are addressed, and all integration tests run successfully against the new testing environment (cardano-testnet + CLB). It's nice to see that both options are briefly compared in the docs, but ideally we should also include benchmarks and a clear description of any shortcomings of CLB (if any), so that users are able to make an informed decision.
As for the other changes (new tests, updated documentation, etc.), let's cherry-pick them into a separate PR.
Regarding the CLB-specific changes, I would suggest merging them into a separate upstream branch for now, until the aforementioned concerns are addressed.
| # it stopped working for later commits on main somehow, so here is the merge | ||
| # commit for branch that brings support for ogmios 6.8.0 | ||
| url = "github:mlabs-haskell/cardano.nix?rev=422b9398d7a7ca1f109900781c7d029db107c8c0"; |
There was a problem hiding this comment.
It is probably because they have removed support for multiple versions in more recent commits - we should make our nix configuration compatible with the latest cardano-nix.
| @@ -17,6 +17,7 @@ | |||
| "blockfrost-local-test": "source ./test/blockfrost-local.env && spago run --main Test.Ctl.Blockfrost.Contract", | |||
| "unit-test": "spago run --main Test.Ctl.Unit", | |||
| "testnet-test": "spago run --main Test.Ctl.Testnet", | |||
| "bet-ref-test": "spago run --main Test.Ctl.BetRef", | |||
There was a problem hiding this comment.
Let's make this test suite part of the testnet-test. I think it's nice to have a more involved example assuming it is stable and takes reasonable time to execute.
| end'' <- getFieldOptional o "end" | ||
| end <- case end'' of | ||
| Nothing -> pure Nothing | ||
| Just end' -> | ||
| if isNull end' then pure Nothing | ||
| else Just <$> decodeEraSummaryTime end' |
There was a problem hiding this comment.
| end'' <- getFieldOptional o "end" | |
| end <- case end'' of | |
| Nothing -> pure Nothing | |
| Just end' -> | |
| if isNull end' then pure Nothing | |
| else Just <$> decodeEraSummaryTime end' | |
| end <- traverse decodeEraSummaryTime =<< getFieldOptional' o "end" |
Also the comment is obsolete now.
| envP = Object.fromFoldable | ||
| [ "TMPDIR" /\ workdir | ||
| ] | ||
| env' = Object.fromFoldable | ||
| [ "CARDANO_CLI" /\ "cardano-cli" | ||
| , "CARDANO_NODE" /\ "cardano-node" | ||
| , "TMPDIR" /\ workdir | ||
| ] | ||
| opts = defaultSpawnOptions | ||
| { cwd = Just workdir | ||
| , env = Just $ Object.union env' env | ||
| , env = Just $ Object.union envP $ Object.union env env' | ||
| , detached = true |
There was a problem hiding this comment.
Why is this rewrite necessary?
This PR is done as the main part of Milestone 5 of the Catalyst project PSM(CLB).
List of features:
@errfrom I am not sure we have to merge this into CTL in the long-term perspective as we discussed we want to continue further splitting it up into smaller pieces. This way I think the whole testnet suite should be a separate package.
Regarding the original testnet tests - as CHANGELOG.md says some are red. They are out of the scope, but I would like to figure out what is wrong there if I have a couple of spare hours.
Pre-review checklist
make format)templates/ctl-scaffold) has been updated## Unreleasedheader, using the appropriate sub-headings (### Added,### Changed,### Removed,### Fixed), and the links to the appropriate issues/PRs have been included