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

Add dag-scope tests #49

Closed
wants to merge 2 commits into from
Closed

Add dag-scope tests #49

wants to merge 2 commits into from

Conversation

guanzo
Copy link
Contributor

@guanzo guanzo commented May 15, 2023

TODO:

  • Add a fixture that supports comprehensive car-scope tests.
  • Rename car-scope to dag-scope once lassie supports it.

How I generated the fixture

#!/bin/bash

# set kubo config "Internal.UnixFSShardingSizeThreshold": "10B"

mkdir -p dir/sub1
echo "abc123" > dir/sub1/hello.txt
mkdir -p dir/sub2
echo "xyz456" > dir/sub2/hello.txt

ROOT_CID=$(ipfs add -Qr --chunker=size-5 dir)

ipfs dag export ${ROOT_CID} > ./t0118-ipip-402-scope.car

The point of 2 subdirectories is that if the request is for one of them (/sub1/hello.txt), than the other (/sub2) should not appear in the CAR.

@guanzo guanzo marked this pull request as ready for review May 26, 2023 22:28
@guanzo guanzo changed the title [WIP] add car-scope tests Add dag-scope tests May 27, 2023
@laurentsenta
Copy link
Contributor

Adding related contributors for review, the code looks fine to me;
Could you rebase over the latest main? It looks like it shows the car tooling changes that are already in main.

@guanzo
Copy link
Contributor Author

guanzo commented May 29, 2023

Had trouble doing a clean rebase so force pushed instead 😅

@galargh
Copy link
Contributor

galargh commented May 30, 2023

The tests haven't been running on this PR because we had a wrong trigger configured for the job. This is being fixed in #60

In the meantime, I pushed https://github.com/ipfs/gateway-conformance/tree/guanzo-main so that we get CI runs for the HEAD of this PR. Unfortunately, it looks like the newly added tests aren't passing when running against kubo-master. See https://github.com/ipfs/gateway-conformance/actions/runs/5119205028/attempts/1#summary-13852714693

@guanzo
Copy link
Contributor Author

guanzo commented May 30, 2023

The failures are due to kubo not "returning intermediate blocks from the CID root to the path terminus as part of the returned CAR file" as mentioned in this PR. ipfs/specs#402. Not sure how we want to proceed as idk when kubo will implement that.

@galargh
Copy link
Contributor

galargh commented May 31, 2023

We can skip the some expects like this:

args: -skip 'TestGatewayCar/GET_response_for_application/vnd.ipld.car/Header_Content-Length'

* add test for missing dag-scope parameter
@guanzo
Copy link
Contributor Author

guanzo commented May 31, 2023

I can skip the CAR cid checks but that basically skips all tests added by this PR 😆 . We can do that or skip the intermediate cid checks for now.

@hacdias
Copy link
Member

hacdias commented Jun 1, 2023

I'm closing this and we'll continue on #56. Thanks @guanzo for your contribution on this. Changes are included in #56.

@hacdias hacdias closed this Jun 1, 2023
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.

4 participants