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

Round-trip storage/retrieval test #229

Merged
merged 4 commits into from
May 8, 2020

Conversation

shannonwells
Copy link
Contributor

@shannonwells shannonwells commented May 6, 2020

Motivation

We need a storage-retrieval round trip integration test. Closes #74

Change Summary:

  • make a test that combines the storage and retrieval market integration tests
  • move TestPeerResolver to shared testutils so it can be used more widely
  • make everybody using LoadUnixFSFile have to specify the path to their fixture file, starting from package root, so it can be used more easily by different packages.
  • export AllSelector
  • please the linter with a bunch of nitpicky changes

More detail about the integration test:
This is essentially the combined storage and retrieval integration tests, with most assertions removed and the remaining ones converted to requires. For retrieval code, the integration test code is converted to use the test harness method in storage integration test, in particular so the LibP2PTestData can be shared from the storage to the retrieval harness, as well as other things such as the Payload CID, so that the stored file can be retrieved.

* move TestPeerResolver to shared testutils
* make everybody specify the path to their fixture file starting from root, so >1 package can use LoadUnixFSFile
* export AllSelector
* please the linter
@shannonwells shannonwells force-pushed the feat/storage-retrieval-roundtrip-#74 branch from 7f53f8e to acf443b Compare May 7, 2020 00:03
Copy link
Contributor

@ingar ingar left a comment

Choose a reason for hiding this comment

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

Looks good, probably there will be some code extraction once #192 lands...

@@ -7,7 +7,7 @@ import (
"github.com/ipld/go-ipld-prime/traversal/selector/builder"
)

func allSelector() ipld.Node {
func AllSelector() ipld.Node {
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like while you are exporting this, this is code snippet is used all over the place in tests and in the storage market...

shared_testutil/mocknet.go Outdated Show resolved Hide resolved
@codecov-io
Copy link

Codecov Report

Merging #229 into master will increase coverage by 0.41%.
The diff coverage is 70.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #229      +/-   ##
==========================================
+ Coverage   66.03%   66.43%   +0.41%     
==========================================
  Files          40       39       -1     
  Lines        2269     2237      -32     
==========================================
- Hits         1498     1486      -12     
+ Misses        657      636      -21     
- Partials      114      115       +1     
Impacted Files Coverage Δ
storagemarket/types.go 33.34% <ø> (ø)
retrievalmarket/impl/client.go 67.89% <50.00%> (ø)
retrievalmarket/impl/provider.go 70.91% <100.00%> (ø)
storagemarket/impl/clientutils/clientutils.go 78.95% <100.00%> (-2.87%) ⬇️
...oragemarket/impl/providerstates/provider_states.go 85.72% <100.00%> (-0.43%) ⬇️
retrievalmarket/types.go 37.50% <0.00%> (+11.03%) ⬆️
retrievalmarket/common.go 71.43% <0.00%> (+71.43%) ⬆️

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 dde200e...f3d7659. Read the comment docs.

@shannonwells shannonwells requested a review from ingar May 7, 2020 23:52
Copy link
Contributor

@ingar ingar left a comment

Choose a reason for hiding this comment

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

Thanks

@shannonwells shannonwells merged commit 32d301b into master May 8, 2020
@shannonwells shannonwells deleted the feat/storage-retrieval-roundtrip-#74 branch May 18, 2020 22:15
@dirkmc dirkmc mentioned this pull request Oct 4, 2021
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.

Roundtrip Storage/Retrieval Market Integration Test
3 participants