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

feat: dedicated target per gateway specification and smaller-range groups #79

Merged
merged 7 commits into from
Jun 19, 2023

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Jun 16, 2023

Close #74. Close #78. This PR does the following:

  • Defines the existing specs in https://specs.ipfs.tech/http-gateways/
  • Some of the specs are decoupled in smaller sub-specifications. They can be enabled and disabled individually.
    • E.g. +TrustlessGateway -TrustlessGatewayIPNS will run CARs and RAW tests.
  • Moves files around to make them more or less match the current specifications.
  • Adds new tests for /ipfs/{cid}?format=raw
  • @laurentsenta I kept in comment the name of the files were tests came from such that you can still go check if all tests were ported from sharness.
  • Unexported Run such that all tests are attached to some specification.

What this PR is not:

  • A perfect separation of tests per spec. There's some tests that are ambiguous and it's most likely because we always assumed "oh, it's Kubo and it can do everything".
  • Did not change the name of the fixture files.

Some notes:


I will also need some feedback here from @galargh @lidel @laurentsenta. Ideally, we should have all specifications defined and some (all?) would run by default. Then the user can tinker with it.

I think the most complicated part is defining subsets. For example: Trustless Gateway is a subset of the Path Gateway. Therefore, if I run the tests for the Path Gateway, I should also be running the Trustless Gateway tests, and so on and so forth.

@hacdias
Copy link
Member Author

hacdias commented Jun 16, 2023

The PR is ready to looked at. Note that I updated the PR description with all the new information. It seems that the tests are stuck "provisioning Kubo gateway" and I'm not sure what's wrong as I did not change any of the fixtures.

@hacdias hacdias marked this pull request as ready for review June 16, 2023 14:01
Copy link
Contributor

@laurentsenta laurentsenta left a comment

Choose a reason for hiding this comment

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

Code LGTM, investigating why CI breaks.

lidel
lidel previously requested changes Jun 16, 2023
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Thank you @hacdias, lgtm!

I've dropped some unrelated TODOs for future PRs (things I've noticed while looking at this PR), but as for this PR, it's only missing docs (outline of mvp readme below).

Ideally, we should have all specifications defined and some (all?) would run by default. Then the user can tinker with it.

Yes, by default we run all, and then user can adjust, if required.

I think this PR should also include section in README about this.
Questions we want to have answered there:

  • what is run by default (all mature tests)
  • opt-outs
    • how user can skip specific test group (example: disable subdomain and dnslink gateways)
    • how user can skip individual test (example: -skip is used in Kubo CI here but not documented anywhere in readme atm, and Saturn folks thought it is not possible)
  • how user can disable all tests and run only specific group
    • including example how to run only the trustless raw+car but without ipns will be enough

tests/dnslink_gateway_test.go Outdated Show resolved Hide resolved
tests/dnslink_gateway_test.go Outdated Show resolved Hide resolved
tests/dnslink_gateway_test.go Outdated Show resolved Hide resolved
tests/dnslink_gateway_test.go Outdated Show resolved Hide resolved
tests/path_gateway_unixfs_test.go Outdated Show resolved Hide resolved
tests/subdomain_gateway_ipfs_test.go Outdated Show resolved Hide resolved
tests/subdomain_gateway_ipfs_test.go Outdated Show resolved Hide resolved
tests/subdomain_gateway_ipfs_test.go Outdated Show resolved Hide resolved
tests/trustless_gateway_raw_test.go Outdated Show resolved Hide resolved
tooling/specs/specs.go Show resolved Hide resolved
@hacdias hacdias requested a review from lidel June 19, 2023 08:53
@hacdias hacdias merged commit 6dd1aaf into main Jun 19, 2023
@hacdias hacdias deleted the specs branch June 19, 2023 08:59
@BigLep BigLep mentioned this pull request Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Move /ipfs/cid/sub?format=raw to path gateway Add dedicated compliance targets for Trustless Gateway
3 participants