Move network harness into lntest package #425
Conversation
jimpo
force-pushed
the
integration-testing
branch
2 times, most recently
from
c34c478 to
ae9b60f
Compare
jimpo
force-pushed
the
integration-testing
branch
from
ae9b60f to
4d1534b
Compare
There was a problem hiding this comment.
I dig the refactor! After this, the package can now be re-used in other projects which need to script the interaction of lnd nodes. Most of my comments a pretty minor, the notable ones are: we now manually the the number of confirmations for a transaction, and about restoring the prior OnTxAccepted callback.
lnd_test.go
Outdated
There was a problem hiding this comment.
Odd line wrapping here. Not replicated in the existing codebase.
There was a problem hiding this comment.
How would you format this declaration to not violate the 80 char limit?
lnd_test.go
Outdated
There was a problem hiding this comment.
Sending over a raw channel here which is then passed into the lnd harness seems like a step backwards. We can still re-use the same callback method added as we'll still start the btcd harness before we start the lnd harness.
lntest/doc.go
Outdated
lntest/harness.go
Outdated
lntest/harness.go
Outdated
There was a problem hiding this comment.
Comment should be updated, this no longer takes the extra command line flags directly.
lntest/harness.go
Outdated
There was a problem hiding this comment.
godoc comment should document the semantics of lndArgs. Based on my earlier comment, I don't think the seenTxns arg is needed at all.
lntest/node.go
Outdated
There was a problem hiding this comment.
Note that in master, we now manually set defaultnumconfs.
lntest/node.go
Outdated
lnd_test.go
Outdated
There was a problem hiding this comment.
This commit is out of order FWIW (it wouldn't compile by itself).
There was a problem hiding this comment.
I think GitHub just orders the commits weird (by date instead of actually in order). The dates are screwed up because I had to cherry-pick a bunch from another branch.
This creates a new nodeConfig struct for the node in the lntest package in order to decouple lntest from the main package.
This is preferable to calling Shutdown on the node directly so that the harness manages the entire lifecycle of an lnd process.
jimpo
force-pushed
the
integration-testing
branch
2 times, most recently
from
1f30c11 to
20ee9d8
Compare
Creates an lntest package for testing utilities. For now it just has the NetworkHarness and HarnessNode structs. Some minor refactors to the harness as well.