Test improvements #814
Test improvements #814
Conversation
| @@ -468,7 +468,7 @@ func TestBeaconSimple(t *testing.T) { | |||
| bt.nodes[i].handler.Lock() | |||
| started := bt.nodes[i].handler.started | |||
| bt.nodes[i].handler.Unlock() | |||
| require.False(t, started, "handler %d has started?", i) | |||
| require.True(t, started, "handler %d has started?", i) | |||
There was a problem hiding this comment.
Im not 100% sure anymore but I think this test was to make sure none started before genesis time - here it should not have started since genesis is at "now() + 2s" and current time is only "now() + 1s". What am i missing ?
There was a problem hiding this comment.
I checked every piece of code looking for the started var. I could not find any place where that var is set to true. I made a change to set it to true when calling the function StartBeacon().
| gp, err := controlClient.InitDKGLeader(d.n, d.thr, d.period, d.catchupPeriod, testDkgTimeout, nil, secret, testBeaconOffset) | ||
| d.t.Log("[RunDKG] Leader init") | ||
|
|
||
| // TODO: Control Client needs every single parameter, not a protobuf type. This means that it will be difficult to extend |
There was a problem hiding this comment.
Well actually there is the control client struct which is only a wrapper around the protobuf - we could use directly here the protobuf and send it via the generic client.
|
Ok but nonetheless in this part of the test the beacon should NOT have
started I believe, do you agree ? It should only be from genesis time that
this variable is set to true no?
…On Thu, 23 Sep 2021, 12:29 Emmanuel, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In chain/beacon/node_test.go
<#814 (comment)>:
> @@ -468,7 +468,7 @@ func TestBeaconSimple(t *testing.T) {
bt.nodes[i].handler.Lock()
started := bt.nodes[i].handler.started
bt.nodes[i].handler.Unlock()
- require.False(t, started, "handler %d has started?", i)
+ require.True(t, started, "handler %d has started?", i)
I checked every piece of code looking for the started var. I could not
find any place where that var is set to true. I made a change to set it to
true when calling the function StartBeacon().
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#814 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AATSFC3LEOSASQ7SFUCKCG3UDMFSHANCNFSM5ESB3RYQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
I think we should create a new flag to indicates the beacon is running or not. I have created three flags: started, running and stopped. I want to detect when I gave the order to start, but it is still not running as the genesis time has not passed. Please check the last commit! |
|
Some notes about CI:
|
emmanuelm41
force-pushed
the
test-improvements
branch
from
3687687
to
0ec2ae7
Compare
In the PR, test improvements where applied. A list of them is write below: