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

Fix restart issue with deal transferring #381

Merged
merged 1 commit into from
Aug 29, 2020
Merged

Conversation

aarshkshah1992
Copy link
Collaborator

Closes #378 .

@aarshkshah1992
Copy link
Collaborator Author

@hannahhoward Let me know if we need to test the restarts after more events.

@hannahhoward
Copy link
Collaborator

@aarshkshah1992 I would check ClientEventDealPublished and ClientEventDealActivated as well, though you can do so in a seperate ticket. That will also probably require making sure the node methods to not return immediately, as I imagine the final states are moved through in quick sucession during test (in a real environment, it's not quick)

Copy link
Collaborator

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

Please run make prepare-pr before merging to pass the docs-check

@hannahhoward
Copy link
Collaborator

@aarshkshah1992 also TestMakeDealNonBlocking being flaky is a known issue -- if you want to look at that in a seperate ticket you're welcome to.

@codecov-commenter
Copy link

Codecov Report

Merging #381 into master will decrease coverage by 0.16%.
The diff coverage is 20.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #381      +/-   ##
==========================================
- Coverage   61.47%   61.32%   -0.15%     
==========================================
  Files          42       43       +1     
  Lines        2873     2882       +9     
==========================================
+ Hits         1766     1767       +1     
- Misses        956      964       +8     
  Partials      151      151              
Impacted Files Coverage Δ
storagemarket/impl/client.go 2.89% <0.00%> (-0.01%) ⬇️
storagemarket/impl/clientstates/client_fsm.go 100.00% <ø> (ø)
storagemarket/impl/provider.go 3.00% <0.00%> (-0.01%) ⬇️
...oragemarket/impl/providerstates/provider_states.go 84.72% <100.00%> (+0.07%) ⬆️
storagemarket/types.go 0.00% <0.00%> (ø)

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 30e8f6d...c3f88aa. Read the comment docs.

@aarshkshah1992
Copy link
Collaborator Author

Created issue to track the remaining tests:

filecoin-project/lotus#3443

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.

Deal restarts fails if Storage Client crashes in the StorageDealTransferring state
3 participants