-
Notifications
You must be signed in to change notification settings - Fork 289
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
test: backports knuu optimizations from 3493 #3515
Changes from all commits
ecc18ea
60de468
c973daa
1c53b71
93d5a4e
248687e
01b9203
92cec9f
9507488
7b8ad0a
685ee9d
43fb5ac
7d628d5
33fa9cd
8831859
566e283
1772eb4
c64c811
1515dea
2df5bf3
fb66838
3b6fbd0
102272e
7185d2d
9f19d68
df9107f
f7a0f81
c8fc94d
77a5b4f
6f25001
bb65e2b
6aab4b4
c62383b
fee3c03
8fc2775
102406e
ed3c96c
5d72549
817816c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -221,11 +221,15 @@ func (n *Node) Init(genesis *types.GenesisDoc, peers []string, configOptions ... | |
return fmt.Errorf("writing address book: %w", err) | ||
} | ||
|
||
if err := n.Instance.AddFolder(nodeDir, remoteRootDir, "10001:10001"); err != nil { | ||
return fmt.Errorf("copying over node %s directory: %w", n.Name, err) | ||
err = n.Instance.Commit() | ||
if err != nil { | ||
return fmt.Errorf("committing instance: %w", err) | ||
} | ||
|
||
return n.Instance.Commit() | ||
if err = n.Instance.AddFolder(nodeDir, remoteRootDir, "10001:10001"); err != nil { | ||
return fmt.Errorf("copying over node %s directory: %w", n.Name, err) | ||
} | ||
return nil | ||
} | ||
|
||
// AddressP2P returns a P2P endpoint address for the node. This is used for | ||
|
@@ -296,10 +300,23 @@ func (n Node) Client() (*http.HTTP, error) { | |
} | ||
|
||
func (n *Node) Start() error { | ||
if err := n.Instance.Start(); err != nil { | ||
if err := n.StartAsync(); err != nil { | ||
return err | ||
} | ||
if err := n.WaitUntilStartedAndForwardPorts(); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no more proxy instead of port forward? or that will come later? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah good point. What order should this PR be merged in respect to the proxying ports PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
return err | ||
} | ||
return nil | ||
} | ||
|
||
func (n *Node) StartAsync() error { | ||
if err := n.Instance.StartAsync(); err != nil { | ||
return err | ||
} | ||
return nil | ||
} | ||
|
||
func (n *Node) WaitUntilStartedAndForwardPorts() error { | ||
staheri14 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err := n.Instance.WaitInstanceIsRunning(); err != nil { | ||
return err | ||
} | ||
|
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.
So we are supposed to add the folder after committing now?
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.
AFAIK, yes, it is the case.
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.
The instances do not differ except for the files we add there.
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 best way to keep track of important details such as this @smuu? we could add a code comment above, but I imagine this would also be important to know when creating tests for node as well
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've created a tracking issue for this #3601