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.

Sign up for GitHub

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

Jump to bottom

testing(dot/sync): create dot sync unit tests #2261

Merged

edwardmack merged 89 commits into development from ed/sync_unit_tests

Jun 13, 2022

Member

edwardmack commented Feb 1, 2022 •

edited

Loading

Changes

Create unit tests for dot/sync packages: chain_processor_test.go, chain_sync_test.go, disjoint_block_set_test.go, massage_test.go, outliers_test.go, syncer_test.go, tip_syncer_test.go
tparallel linter
remove nil block check in handleBlock

Tests

go test ./dot/sync/... -v

Issues

closes Create dot/sync unit tests #2024

Primary Reviewer

@timwu20

edwardmack requested review from EclesioMeloJunior, jimjbrettj, kishansagathiya, noot, qdm12 and timwu20 as code owners

February 1, 2022 22:33

edwardmack force-pushed the ed/sync_tests_to_integration branch from 2b46760 to bdee8ea Compare

February 4, 2022 15:56

edwardmack force-pushed the ed/sync_unit_tests branch 2 times, most recently from 72f8a3d to b346712 Compare

February 4, 2022 17:04

codecov bot commented Feb 4, 2022 •

edited

Loading

Codecov Report

Merging #2261 (35962eb) into development (6d22c23) will increase coverage by 4.59%.
The diff coverage is 55.49%.

@@               Coverage Diff               @@
##           development    #2261      +/-   ##
===============================================
+ Coverage        57.34%   61.94%   +4.59%     
===============================================
  Files              215      215              
  Lines            28417    28407      -10     
===============================================
+ Hits             16296    17596    +1300     
+ Misses           10442     9063    -1379     
- Partials          1679     1748      +69

Flag	Coverage Δ
unit-tests	`61.94% <55.49%> (+4.59%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
dot/sync/disjoint_block_set.go	`75.58% <ø> (+75.58%)`	⬆️
dot/sync/test_helpers.go	`0.00% <ø> (ø)`
lib/runtime/constants.go	`0.00% <ø> (ø)`
lib/runtime/test_helpers.go	`7.94% <0.00%> (-0.65%)`	⬇️
dot/peerset/peerset.go	`49.34% <42.85%> (-0.23%)`	⬇️
dot/state/epoch.go	`49.53% <50.00%> (+0.23%)`	⬆️
dot/sync/chain_sync.go	`76.12% <93.42%> (+76.12%)`	⬆️
dot/sync/chain_processor.go	`97.60% <100.00%> (+97.60%)`	⬆️
dot/sync/outliers.go	`96.15% <100.00%> (+96.15%)`	⬆️
dot/sync/syncer.go	`91.25% <100.00%> (+91.25%)`	⬆️
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2fa5d8a...35962eb. Read the comment docs.

edwardmack force-pushed the ed/sync_unit_tests branch 2 times, most recently from e8d4957 to 1d3de9d Compare

February 4, 2022 20:26

noot reviewed

View reviewed changes

dot/network/service.go Outdated Show resolved Hide resolved

noot reviewed

View reviewed changes

dot/sync/syncer_test.go Outdated Show resolved Hide resolved

edwardmack force-pushed the ed/sync_tests_to_integration branch 2 times, most recently from 324c1f6 to b6073f0 Compare

March 11, 2022 21:53

edwardmack force-pushed the ed/sync_unit_tests branch from 1d3de9d to 99541e2 Compare

March 14, 2022 23:36

Contributor

timwu20 commented Apr 5, 2022

@edwardmack looks like a mock is failing on an assertion.

edwardmack marked this pull request as draft

April 6, 2022 20:56

edwardmack changed the title ~~testing (dot/sync) create dot sync unit tests~~ testing(dot/sync): create dot sync unit tests

edwardmack marked this pull request as ready for review

April 11, 2022 14:51

timwu20 requested changes

View reviewed changes

Contributor

timwu20 left a comment

Fantastic work Ed! I added a few suggestions. I see a lot of improvement in this PR. 🚀

dot/sync/bootstrap_syncer_test.go Outdated Show resolved Hide resolved

dot/sync/chain_processor_test.go Outdated Show resolved Hide resolved

dot/sync/chain_sync_test.go Outdated Show resolved Hide resolved

dot/sync/chain_sync_test.go Outdated Show resolved Hide resolved

dot/sync/disjoint_block_set_test.go Outdated Show resolved Hide resolved

dot/sync/disjoint_block_set_test.go Outdated Show resolved Hide resolved

dot/sync/message_test.go Outdated Show resolved Hide resolved

dot/sync/syncer_test.go Show resolved Hide resolved

EclesioMeloJunior requested changes

View reviewed changes

dot/sync/bootstrap_syncer_test.go Outdated Show resolved Hide resolved

qdm12 reviewed

View reviewed changes

dot/sync/chain_processor.go Outdated Show resolved Hide resolved

dot/sync/chain_processor_test.go Outdated Show resolved Hide resolved

dot/sync/chain_processor_test.go Outdated Show resolved Hide resolved

dot/sync/chain_processor_test.go Outdated Show resolved Hide resolved

dot/sync/chain_processor_test.go Outdated Show resolved Hide resolved

dot/sync/bootstrap_syncer_test.go Outdated Show resolved Hide resolved

dot/sync/chain_sync_integeration_test.go Outdated Show resolved Hide resolved

dot/sync/chain_sync_integeration_test.go Outdated Show resolved Hide resolved

dot/sync/chain_sync_test.go Outdated Show resolved Hide resolved

lib/runtime/interface.go Outdated Show resolved Hide resolved

qdm12 reviewed

View reviewed changes

dot/sync/chain_sync_integeration_test.go Outdated Show resolved Hide resolved

dot/sync/chain_sync_integeration_test.go Outdated Show resolved Hide resolved

dot/sync/disjoint_block_set_test.go Outdated Show resolved Hide resolved

dot/sync/disjoint_block_set_test.go Outdated Show resolved Hide resolved

dot/sync/disjoint_block_set_test.go Outdated Show resolved Hide resolved

dot/sync/disjoint_block_set_test.go Outdated Show resolved Hide resolved

dot/sync/disjoint_block_set_test.go Outdated Show resolved Hide resolved

dot/sync/disjoint_block_set_test.go Outdated Show resolved Hide resolved

dot/sync/disjoint_block_set_test.go Outdated Show resolved Hide resolved

dot/sync/disjoint_block_set_test.go Outdated Show resolved Hide resolved

qdm12 reviewed

View reviewed changes

dot/sync/bootstrap_syncer_test.go Outdated Show resolved Hide resolved

Member Author

edwardmack commented Apr 26, 2022

This should be ready for a re-review. Note that I've added //nolint:govet to places where fields test struct was replaced with disjointBlockSet directly since disjointBlockSet contains a sync.RWMutex. Let me know if there is a better way to handle this.

EclesioMeloJunior requested changes

View reviewed changes

Member

EclesioMeloJunior left a comment

LGTM! Just small changes

dot/sync/bootstrap_syncer_test.go Show resolved Hide resolved

dot/sync/chain_sync.go Outdated Show resolved Hide resolved

lib/runtime/interface.go Outdated Show resolved Hide resolved

timwu20 approved these changes

View reviewed changes

Contributor

timwu20 left a comment

Really good work Ed!

qdm12 reviewed

View reviewed changes

dot/sync/tip_syncer_integeration_test.go Outdated Show resolved Hide resolved

dot/sync/bootstrap_syncer_test.go Outdated Show resolved Hide resolved

dot/sync/bootstrap_syncer_test.go Outdated Show resolved Hide resolved

dot/sync/bootstrap_syncer_test.go Show resolved Hide resolved

dot/sync/chain_processor_test.go Outdated Show resolved Hide resolved

dot/sync/chain_processor_test.go Outdated Show resolved Hide resolved

dot/sync/chain_processor_test.go Outdated Show resolved Hide resolved

dot/sync/chain_processor_test.go Outdated Show resolved Hide resolved

dot/sync/chain_processor_test.go Outdated Show resolved Hide resolved

dot/sync/chain_processor_test.go Outdated Show resolved Hide resolved

edwardmack and others added 24 commits

June 10, 2022 17:49


          fix linter issues

0f67d1f


          lint

6d079bd


          address PR comments

2085ccc


          address lint issues

a2e5811


          fix testing errors

4b579d1


          add to chainProcessor handleBlock tests

fca4b4e


          improve chain processor tests

8ab179d


          fix tests, address comments

f9121d2


          WIP - save commit

2bc5b97


          save commit - WIP - address comments

d0cd59f


          address PR comments


          add todo comment to fix time.Sleep

be4ed81


          removed sleep timers in tests

d0c9f48


          address PR comments

8cb0f3d


          fix merge conflicts,

f8da8f9


          revert polkadotjs test to match package.json version

f166f6e


          testing hash for test

52c79d8


          test stuff

dc90c59


          add to test coverage


          address PR comments

cfb3b53


          address PR comments

2d56af6


          Rework Test_chainSync_setPeerHead

363c53e


          Fix controller usage in subtests

2f64578


          Fix Service Start to not launch goroutines

66fb539

edwardmack force-pushed the ed/sync_unit_tests branch from 70f8989 to 66fb539 Compare

June 10, 2022 21:52

edwardmack added 3 commits

June 10, 2022 17:56


          remove unused functions

fcd5eac


          start s.chainSync.start as go routine

0deee97


          add done channel to test

35962eb

edwardmack merged commit 16313f8 into development

edwardmack deleted the ed/sync_unit_tests branch

June 13, 2022 16:31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

qdm12 qdm12 approved these changes

noot noot left review comments

timwu20 timwu20 approved these changes

EclesioMeloJunior EclesioMeloJunior approved these changes

jimjbrettj Awaiting requested review from jimjbrettj jimjbrettj is a code owner

kishansagathiya Awaiting requested review from kishansagathiya kishansagathiya is a code owner

Labels

None yet