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
Coverage for facade #5237
Coverage for facade #5237
Conversation
facade/nodeFacade_test.go
Outdated
nf, _ := NewNodeFacade(arg) | ||
arg := createMockArguments() | ||
arg.Node = &mock.NodeStub{ | ||
GetAccountCalled: func(address string, _ api.AccountQueryOptions) (api.AccountResponse, api.BlockInfo, error) { |
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 address isn't used either
t.Parallel() | ||
|
||
nf, _ := NewNodeFacade(createMockArguments()) | ||
require.NoError(t, nf.Close()) |
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.
it seems like the context created on the constructor isn't used anywhere. might remove it and the cancel function
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.
indeed, fixed
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## rc/v1.6.0 #5237 +/- ##
=============================================
- Coverage 79.59% 79.47% -0.13%
=============================================
Files 680 680
Lines 88012 88010 -2
=============================================
- Hits 70053 69944 -109
- Misses 12837 12912 +75
- Partials 5122 5154 +32
☔ View full report in Codecov by Sentry. |
@@ -68,8 +68,6 @@ type nodeFacade struct { | |||
accountsState state.AccountsAdapter | |||
peerState state.AccountsAdapter | |||
blockchain chainData.ChainHandler | |||
ctx context.Context |
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.
👍
golang says: we should not store contexts
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.
System test passed.
@@ Log scanner @@
facade_tests
================================================================================
- Known Warnings 10
- New Warnings 3
- Known Errors 1
- New Errors 1
- Panics 0
================================================================================ - block hash does not match 11250
- wrong nonce in block 4000
- miniblocks does not match 0
- num miniblocks does not match 0
- miniblock hash does not match 0
- block bodies does not match 0
- receipts hash missmatch 1
================================================================================ - No jailed nodes on the testnet
================================================================================
Reasoning behind the pull request
Proposed changes
Testing procedure
Pre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
feat
branch created?feat
branch merging, do all satellite projects have a proper tag insidego.mod
?