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

test: cli: unit tests for sync related commands #8080

Merged
merged 10 commits into from
Feb 27, 2022
Merged

Conversation

TheDivic
Copy link
Contributor

@TheDivic TheDivic commented Feb 11, 2022

Proposed Changes

This PR contains unit tests for CLI commands from the sync category e.g. lotus sync *.
Everything is covered except lotus sync wait since it's tricky to mock.

Additional Info

  • ⚠️ This PR depends on helper functions and mocks implemented in this PR, and that needs to be merged first. That's why it seems big and it's in DRAFT status, but as soon as the other PR is merged into master, I'll merge those changes into this branch and this will be much easier to review.

Checklist

Before you mark the PR ready for review, please make sure that:

  • All commits have a clear commit message.
  • The PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, INTERFACE BREAKING CHANGE, CONSENSUS BREAKING, build, chore, ci, docs,perf, refactor, revert, style, test
    • area: api, chain, state, vm, data transfer, market, mempool, message, block production, multisig, networking, paychan, proving, sealing, wallet, deps
  • This PR has tests for new functionality or change in behaviour
  • If new user-facing features are introduced, clear usage guidelines and / or documentation updates should be included in https://lotus.filecoin.io or Discussion Tutorials.
  • CI is green

deals_retry_deal_no_funds_test.go:TestDealsRetryLackOfFunds seems to be
flaky, sometimes it passes and sometimes it doesn't.
The first suspect for me was time.Sleep(), which I replaced with waiting
until a deal state where you can restart is reached.
@codecov
Copy link

codecov bot commented Feb 12, 2022

Codecov Report

Merging #8080 (e753e7a) into master (5d416de) will increase coverage by 0.02%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8080      +/-   ##
==========================================
+ Coverage   39.91%   39.94%   +0.02%     
==========================================
  Files         666      666              
  Lines       72554    72556       +2     
==========================================
+ Hits        28963    28985      +22     
+ Misses      38553    38503      -50     
- Partials     5038     5068      +30     
Impacted Files Coverage Δ
cli/sync.go 35.79% <85.71%> (+35.79%) ⬆️
chain/actors/builtin/miner/diff.go 48.52% <0.00%> (-10.30%) ⬇️
cli/util.go 70.83% <0.00%> (-8.34%) ⬇️
chain/events/observer.go 71.64% <0.00%> (-6.72%) ⬇️
storage/wdpost_sched.go 75.49% <0.00%> (-5.89%) ⬇️
chain/exchange/peer_tracker.go 66.66% <0.00%> (-4.31%) ⬇️
miner/miner.go 55.08% <0.00%> (-2.96%) ⬇️
chain/gen/gen.go 68.19% <0.00%> (-1.23%) ⬇️
extern/storage-sealing/fsm.go 56.44% <0.00%> (-1.05%) ⬇️
extern/sector-storage/sched.go 84.77% <0.00%> (-0.83%) ⬇️
... and 12 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 5d416de...e753e7a. Read the comment docs.

@brdji brdji marked this pull request as ready for review February 22, 2022 13:35
@brdji brdji requested a review from a team as a code owner February 22, 2022 13:35
@magik6k magik6k merged commit a49ec59 into master Feb 27, 2022
@magik6k magik6k deleted the bloxico/cli-sync-tests branch February 27, 2022 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants