refactor: Node config #2296
refactor: Node config #2296
Conversation
| err = node.Start(ctx) | ||
| require.NoError(t, err) | ||
|
|
||
| <-time.After(5 * time.Second) |
There was a problem hiding this comment.
question: How much value do you think this test provides? Is it not covering something that is covered by pretty much every integration test we have, but with a 5 second cost?
There was a problem hiding this comment.
I wanted to have something similar to the start-binary workflow. I think it would be valuable to catch those errors before that workflow is run.
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
There was a problem hiding this comment.
question: These tests do not appear to actually test their config works, only that it does not error - they (the WithFoo funcs) could be empty funcs for all these tests care. Do you think they are worth keeping and maintaining?
There was a problem hiding this comment.
I've updated the tests. They should make more sense now.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #2296 +/- ##
===========================================
+ Coverage 74.00% 74.12% +0.13%
===========================================
Files 256 258 +2
Lines 25602 25641 +39
===========================================
+ Hits 18945 19006 +61
+ Misses 5342 5307 -35
- Partials 1315 1328 +13
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 11 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
## Relevant issue(s) Resolves sourcenetwork#2295 ## Description This PR refactors the node setup logic into a new `node` package. ## Tasks - [x] I made sure the code is well commented, particularly hard-to-understand areas. - [x] I made sure the repository-held documentation is changed accordingly. - [x] I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in [tools/configs/chglog/config.yml](tools/configs/chglog/config.yml)). - [x] I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ... ## How has this been tested? make test Specify the platform(s) on which this was tested: - MacOS
Relevant issue(s)
Resolves #2295
Description
This PR refactors the node setup logic into a new
nodepackage.Tasks
How has this been tested?
make test
Specify the platform(s) on which this was tested: