-
Notifications
You must be signed in to change notification settings - Fork 84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use cardano-node as a dependency and introduce CardanoNodeSpec #329
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not convinced by this approach esp. as it adds a manual step in the non-nix case but I will go with the flow
nix-shell --arg withoutDevTools true --run 'cabal update' | ||
nix-shell --arg withoutDevTools true --run 'cabal build ${{ matrix.package }}' | ||
nix-shell --pure --arg withoutDevTools true --run 'cabal update' | ||
nix-shell --pure --arg withoutDevTools true --run 'cabal build ${{ matrix.package }}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the meaning of pure
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not use anything from the host environment. I think hai added it to not use git from mac or so..at least he added git, which was seemingly the only thing used from our hosts.
withBFTNode tr cfgA $ \nodeA -> do | ||
withBFTNode tr cfgB $ \nodeB -> do | ||
withBFTNode tr cfgC $ \nodeC -> do | ||
withBFTNode (nodeTracer cfgA) cfgA $ \nodeA -> do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: only pass cfgA
and retrieve tracer from it inside withBFTNode
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, ok, it does not work the way I thought it worked :) Maybe this is more confusing than the previous version 🤔
Add cardano-node / cardano-cli via nix to shell.nix and add installation instructions to non-nix based setup. Also using pure nix-shells in CI
This also asserts the "right" cardano-version is available as we want to avoid false-positives in our integration tests -> see note in code.
a318cdb
to
47f86e2
Compare
@ch1bo I forced push an update to latest master |
This follows-up and will close #178 as it adds the discussed check for
cardano-node
version present to avoid false-positives in case integration tests fail.While at it, I also introduced a
CardanoNodeSpec
and movedwithBFTNode
tohydra-cluster
sCardanoNode
module + some minor moving of test code.