Skip to content

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

@hacdias hacdias requested a review from lidel June 19, 2023 08:53
@hacdias hacdias dismissed lidel’s stale review June 19, 2023 08:57

addressed

@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

4 participants