add go-check workflow, fix gofmt, go vet and staticcheck #417
Conversation
|
No idea why Circle is failing here. Seems to be unrelated to this PR. Some |
gossipsub_spam_test.go
Outdated
| } | ||
| }() | ||
| } | ||
| } | ||
|
|
||
| for err := range errs { | ||
| t.Fatal(err) |
There was a problem hiding this comment.
or just make the calls above t.Errorf. a Fatal here is a bit weird, as the first one will exit the entire test.
There was a problem hiding this comment.
yeah, Errorf is better here.
|
The CircleCI failure is odd, seems it can't checkout the code because of some key issue -- maybe credential rotation? |
| @@ -405,16 +416,18 @@ func TestGossipsubAttackGRAFTDuringBackoff(t *testing.T) { | |||
|
|
|||
| newMockGS(ctx, t, attacker, func(writeMsg func(*pb.RPC), irpc *pb.RPC) { | |||
| // When the legit host connects it will send us its subscriptions | |||
| errs := make(chan error) | |||
There was a problem hiding this comment.
we need to make this buffered or we might end up with stuck goroutines.
| @@ -188,15 +188,17 @@ func TestGossipsubAttackSpamIHAVE(t *testing.T) { | |||
|
|
|||
| newMockGS(ctx, t, attacker, func(writeMsg func(*pb.RPC), irpc *pb.RPC) { | |||
| // When the legit host connects it will send us its subscriptions | |||
| errs := make(chan error) | |||
There was a problem hiding this comment.
this should be buffered i think.
gossipsub_spam_test.go
Outdated
| } | ||
| }() | ||
| } | ||
| } | ||
|
|
||
| for err := range errs { | ||
| t.Fatal(err) |
There was a problem hiding this comment.
yeah, Errorf is better here.
| } | ||
| }() | ||
| } | ||
| } | ||
|
|
||
| for err := range errs { | ||
| t.Fatal(err) |
There was a problem hiding this comment.
yeah, let's make that an Errof.
| } | ||
| }() | ||
| } | ||
| } | ||
|
|
||
| for err := range errs { | ||
| t.Fatal(err) |
|
We might need to refresh the deploy key in the project settings? I don't have admin to do that however, @marten-seemann do you have perms? |
I missed a few readds when I cycled keys due to the codecov incident last week. Remaining repos should have the new keys now |
marten-seemann
force-pushed
the
add-and-fix-go-check
branch
from
5cf742d
to
d2a1ee3
Compare
marten-seemann
force-pushed
the
add-and-fix-go-check
branch
from
d2a1ee3
to
5d12f48
Compare
| Control: &pb.ControlMessage{Graft: graft}, | ||
| }) | ||
|
|
||
| go func() { | ||
| defer close(errs) |
There was a problem hiding this comment.
That will cause duplicate closes of the channel.
| }) | ||
|
|
||
| go func() { | ||
| defer close(errs) |
There was a problem hiding this comment.
This will cause duplicate closes of the channel.
| }) | ||
|
|
||
| go func() { | ||
| defer close(errs) |
There was a problem hiding this comment.
This will cause duplicate closes of the channel.
|
the changes with the error channels seem to have broken some tests... |
|
I think all the new channels should be unnecessary; just use Errorf where the original code used Fatalf :) |
|
@marten-seemann @vyzo what's the status of this PR? |
|
@aschmahmann Due to the resource requirements (especially fds) of the test suite here, we removed this repo from the automation: protocol/.github#51. We can also close this PR. |
Closes #415. Closes #416.
I see that running
go testis more complicated on this repo, so we should probably not move it to GitHub Actions (yet).@coryschwartz did an excellent job fixing all the
go vetandstaticcheckerrors. We should merge these changes, and add thego-checkworkflow.