Remove obsolete code for testing generation of jormungandr-config.yaml #956
Conversation
piotr-iohk
changed the title
piotr-iohk
force-pushed
the
piotr/launch-in-passive-mode-tests
branch
2 times, most recently
from
25e1d3b
to
2b062f0
Compare
piotr-iohk
force-pushed
the
piotr/launch-in-passive-mode-tests
branch
2 times, most recently
from
ab946d2
to
5ea721b
Compare
|
bors try |
tryBuild succeeded |
piotr-iohk
force-pushed
the
piotr/launch-in-passive-mode-tests
branch
from
5ea721b
to
66b20d1
Compare
|
|
||
| spec :: forall t. KnownCommand t => SpecWith (Context t) | ||
| spec = do | ||
| block0H <- runIO $ T.unpack <$> getBlock0HText | ||
| let configForLaunch = "test/data/jormungandr/config-for-passive-launch.yaml" |
There was a problem hiding this comment.
We should use OS-agnostic path separator here to make the reference works fine on all platforms!
import System.FilePath((</>))
let configForLaunch = "test" </> "data" </> ...| address: "/ip4/127.0.0.1/tcp/8081", | ||
| id: "1a563f5c367515f5eadc1c6c690d55320ef578fdd8c2bbe1" | ||
| } | ||
| ] |
There was a problem hiding this comment.
Where's that trusted peer coming from though 😶 ?
Can this list be empty (not sure of Jörmungandr behavior in this scenario)?
Also, I wonder about the potential value of such automated test. To be really interesting, we would have to connect to actual peers and see that we are processing blocks. Yet this requires a running network to connect to. I think that's something we'd better do as a nightly test, very much like the restoration test we used to have.
There was a problem hiding this comment.
Where's that trusted peer coming from though no_mouth ?
From here -> https://github.com/input-output-hk/cardano-wallet/pull/956/files#diff-61049af2693330dca34f6fedf02ab74eR1-R5
(as the config.yaml is used to start the main jormungandr in integration tests -> https://github.com/input-output-hk/cardano-wallet/pull/956/files#diff-6309c3e81be1c6f2739233773ccb247bR52-R54)
Also, I wonder about the potential value of such automated test
The test launches a wallet and a node in a passive mode and connects to a main jormungandr. (Pretty much a Daedalus use case)
This check may be too weak -> https://github.com/input-output-hk/cardano-wallet/pull/956/files#diff-64ca464507dd45f6fad843c0767f1962R81-R85, yet, again, this is what Daedalus is doing. (see: #1009 (comment))
There was a problem hiding this comment.
I see. We can't actually rely on 8081 to always be available on the machine, especially in CI.
This setup requires a bit more effort and is something we'll work out for the nightly tests! I'll open a corresponding ticket on the laboratory to capture this and close this PR if you don't mind ?
There was a problem hiding this comment.
Ok, would it make sense then to rework this PR and leave the first commit actually (without modifications to config.yaml)? It is about removing old test code.
There was a problem hiding this comment.
Hmmmm. But we still do use that code actually. The call to genConfigYaml is essential to make sure we run various executions of Jörmungandr using different state directory.
The public_address part is probably obsolete now however, because this can now be removed entirely from the configuration file and we have no particular use for it.
There was a problem hiding this comment.
Wait a minute.. I think you're right actually 🤔 ... even the storage directory, it isn't taken from the configuration anymore...
There was a problem hiding this comment.
Correct. We are no longer generating config.yaml for jormungandr like we used to.
There was a problem hiding this comment.
Yes, so we can indeed keep that first commit modulo the public_id addition in the configuration file.
Eventually, we should even get rid of:
[ "--secret", testDataDir </> "secret.yaml"
, "--config" , testDataDir </> "config.yaml"
]
which should be done at the call-site and not from this function.
piotr-iohk
force-pushed
the
piotr/launch-in-passive-mode-tests
branch
from
66b20d1
to
9287c5f
Compare
piotr-iohk
changed the title
piotr-iohk
force-pushed
the
piotr/launch-in-passive-mode-tests
branch
from
9287c5f
to
80faee2
Compare
| @@ -80,7 +80,7 @@ prop_combineDefaults | |||
| prop_combineDefaults mStake = do | |||
| combineMetrics mStake Map.empty Map.empty | |||
| === | |||
| (Right $ Map.map (, Quantity 0, 0) mStake) | |||
| Right (Map.map (, Quantity 0, 0) mStake) | |||
|
bors r+ |
956: Remove obsolete code for testing generation of jormungandr-config.yaml r=KtorZ a=piotr-iohk # Issue Number #832 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [ ] I have removed spec related to `config.yaml` generation - [ ] made that our integration tests are using `config.yaml` - [ ] ~added test for `launch` with `--genesis-block-hash` (passive mode)~ # Comments <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Piotr Stachyra <[email protected]>
Build succeeded |
Issue Number
#832
Overview
config.yamlgenerationconfig.yamladded test forlaunchwith--genesis-block-hash(passive mode)Comments