Skip to content
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 common package #5294

Merged
merged 7 commits into from Jun 6, 2023
Merged

Coverage for common package #5294

merged 7 commits into from Jun 6, 2023

Conversation

sstanculeanu
Copy link
Contributor

Reasoning behind the pull request

  • increase code coverage

Proposed changes

  • added unit tests on common package

Testing procedure

  • standard system test

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

  • was the PR targeted to the correct branch?
  • if this is a larger feature that probably needs more than one PR, is there a feat branch created?
  • if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

@sstanculeanu sstanculeanu self-assigned this May 29, 2023
@iulianpascalau iulianpascalau self-requested a review May 29, 2023 12:33
@BeniaminDrasovean BeniaminDrasovean self-requested a review May 29, 2023 12:34
@codecov
Copy link

codecov bot commented May 29, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.22 🎉

Comparison is base (ab04974) 79.93% compared to head (e7da54b) 80.16%.

❗ Current head e7da54b differs from pull request most recent head 3bdea37. Consider uploading reports for the commit 3bdea37 to get more accurate results

Additional details and impacted files
@@              Coverage Diff              @@
##           rc/v1.6.0    #5294      +/-   ##
=============================================
+ Coverage      79.93%   80.16%   +0.22%     
=============================================
  Files            695      695              
  Lines          90066    90070       +4     
=============================================
+ Hits           71993    72203     +210     
+ Misses         12868    12667     -201     
+ Partials        5205     5200       -5     
Impacted Files Coverage Δ
.../softwareVersion/factory/softwareVersionFactory.go 100.00% <100.00%> (ø)
...mmon/statistics/softwareVersion/softwareVersion.go 97.56% <100.00%> (+26.82%) ⬆️
common/validatorInfo/validatorInfoUtils.go 100.00% <100.00%> (+56.25%) ⬆️

... and 15 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

t.Parallel()

invalidPem := `
-----BEGIN PRIVATE KEY for erd1ecdwux5tvanwryhr7cn5l9kc07ayquec7h2jc608kz0ycychzexsj6qw4j-----
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a known private key (e.g. Alice)? Also, this should be validPem, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created it for tests only.. renamed to validPem

common/configParser_test.go Outdated Show resolved Hide resolved
common/configParser_test.go Show resolved Hide resolved
common/configParser_test.go Show resolved Hide resolved
common/converters_test.go Outdated Show resolved Hide resolved

handler.EpochConfirmed(77, 0)
handler.EpochConfirmed(85, 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when we will merge the master -> rc/v1.5.0 -> rc/v1.6.0 this line will become handler.EpochConfirmed(math.MaxUint32, 0).

})
t.Run("flags with == condition should be set, along with all >=", func(t *testing.T) {
t.Parallel()

epoch := uint32(80)
epoch := uint32(85)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when we will merge the master -> rc/v1.5.0 -> rc/v1.6.0 this line will become epoch := uint32(math.MaxUint32).

common/statistics/resourceMonitor_test.go Outdated Show resolved Hide resolved
@@ -30,7 +31,7 @@ func NewSoftwareVersionFactory(
}

// Create returns a software version checker object
func (svf *softwareVersionFactory) Create() (*softwareVersion.SoftwareVersionChecker, error) {
func (svf *softwareVersionFactory) Create() (statistics.SoftwareVersionChecker, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

iulianpascalau
iulianpascalau previously approved these changes May 29, 2023
…into common_tests

# Conflicts:
#	common/enablers/enableEpochsHandler_test.go
iulianpascalau
iulianpascalau previously approved these changes May 31, 2023
Copy link
Collaborator

@gabi-vuls gabi-vuls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

System test passed

@sstanculeanu sstanculeanu merged commit 4e251b2 into rc/v1.6.0 Jun 6, 2023
5 checks passed
@sstanculeanu sstanculeanu deleted the common_tests branch June 6, 2023 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants