feat: add IPIP and TestGroup Metadatas #130

laurentsenta · 2023-08-04T11:23:35Z

Contributes to #123
Related comment: https://github.com/ipfs/gateway-conformance/pull/116/files#r1277390949

To implement Dashboards, we introduce:

The ability to define "human readable" test groups in code, which do not depend on the test structure, these could be used to generate user-friendly tables like the one in public-gateway-checker.
The ability to associate IPIPs reference with a test case and also link to subsections.

Example Output: https://github.com/ipfs/gateway-conformance/actions/runs/5880695313/attempts/1#summary-15947714091
Note how the tests are grouped, and how TrustlessCarEntityBytes links to the corresponding ipip

Todos

Introduce the metadata APIs required
Draft a dashboard output that relies on these new specs
Implement a generator that outputs the groups, versions, and IPIPs

Follow-ups

demonstrate a more scalable generator that relies on turning outputs "queriable" data
demonstrate a reverse-index that goes from an IPIP and list all the tests (when linked from the specs)
When the need arise, we can add IPIP markers to a check (like a single header or payload test)

laurentsenta · 2023-08-16T15:29:47Z

tests/trustless_gateway_car_test.go

@@ -11,6 +12,8 @@ import (
 )

 func TestTrustlessCarPathing(t *testing.T) {
+	tooling.LogTestGroup(t, GroupTrustlessGateway)
+


We define test groups like this

laurentsenta · 2023-08-16T15:30:16Z

tests/trustless_gateway_car_test.go

@@ -39,6 +42,7 @@ func TestTrustlessCarPathing(t *testing.T) {
 						Exactly().
 						InThatOrder(),
 				),
+			IPIP: IPIP402,


We can associate an IPIP to a test, later maybe we'll let users associate to the check itself.

laurentsenta · 2023-08-16T15:30:51Z

tests/trustless_gateway_car_test.go

@@ -374,6 +384,9 @@ func TestTrustlessCarDagScopeAll(t *testing.T) {
 }

 func TestTrustlessCarEntityBytes(t *testing.T) {
+	tooling.LogTestGroup(t, GroupTrustlessGateway)
+	tooling.LogIPIP(t, IPIP402, "4.2.2")


We can associate an ipip + it's section to a group of tests.

IPIPs aren't, in general, that long. I'm not sure if including their section is useful, as they all have more or less the same structure (summary, motivation, design, rationale, etc). Nevertheless, I do like the idea of associating the tests to IPIPs.

💭 It may not be that useful to associate the entire test group to a single IPIP.
IPIPs are usually deltas against existing specs, and there may be IPIPs on top of historical IPIPs too.

We may have such groups in the future, so fine to keep this, but for now a more useful would be to have something like already existing Hint where specific test case can include a list of IPIPs related to the test.

So far we've beein including IPIP number in the Hint, having dedicated func for this would allow us to print more meaningful error message with link to specific IPIP at https://specs.ipfs.tech/ipips/

laurentsenta · 2023-08-16T15:34:52Z

@lidel, @hacdias, @galargh I'm preparing the conformance test suite to generate more advanced dashboards, like the one we find in pinning service compliance and gateway checkers.

The goal here is to get your ack on how we'll associate metadata to tests like test groups and IPIPs references.

The output looks like this:
https://github.com/ipfs/gateway-conformance/actions/runs/5880695313/attempts/1#summary-15947714091
but not that this will change A LOT soon when I can generate more advanced dashboards.

lidel · 2023-08-17T16:55:18Z

Thank you for prototyping this @laurentsenta

I think the main feedback is that IPIPs are too ephemeral to be used as a high level source of truth, avoid mentioning them in high level reports unless they have a dedicated test group.
Usually, we have mandatory and optional Gateway features, and we should frame things around that, and not IPIPs.

IPIPs describe change to the specs, and should be only mentioned when a specific test case fails.

My thinking is:

rule of thumb: we want to hide IPIPs from high level groups and surface them when there is a specific test case instead
- this means people who are interested in high level spec get to the always-up-to-date spec document, and we only surface historic IPIP details only when a related test case fails
for high level groups like "Trustless Gateway" to link to relevant specs https://specs.ipfs.tech/http-gateways/trustless-gateway/
for specific groups, try to link to relevant sections in specs, instead of IPIPs – this will be more future-proof, as there may be new IPIP that changes behavior, and the section will always describe latest behavior
- example: TrustlessCarEntityBytes can now link to permalink at https://specs.ipfs.tech/http-gateways/trustless-gateway/#entity-bytes-request-query-parameter (added since we've merged the IPIP-402)
- 👉 perhaps at the group level, instead of IPIP, have URL, and that URL could point at the best resource available (spec section, or IPIP)
👉 IPIP information is the most valuable at the level of specific test check. This means we want a companion to Hint where we can list numbers of IPIPs that are related to the check, and then we can use this metadata info for generating useful output which links to things in https://specs.ipfs.tech/ipips/
- 💭 perhaps more useful/generic would be HintURLs list, where we could pass multiple URLs related to the test, could be section in specs related to header or parameter, or an IPIP

laurentsenta · 2023-08-22T10:59:21Z

@lidel, thanks for the thorough review and spending the time to clarify!

I'll propose a new API to include your feedbacks regarding specs and ipips.

Regarding the high-level groups: my intent is to create 2 different concepts, "human-readable groups" metadata, and "specs" metadata.

test groups will be used in other projects like public-gateway-checker, and they would be much more flexible. See my comment in feat: generates parts of the dashboard async, using ipfs/gateway-conformance public-gateway-checker#442 (comment). We might merge, split, and rename these frequently.
specs-related metadata will evolve along https://specs.ipfs.tech/ and will be MUCH more constrained. The end goal is to be able to link conformance tests from the specs.

BigLep · 2023-08-22T19:49:00Z

Thanks for sharing. I quickly scanned and saw some of the comments.

From looking at https://github.com/ipfs/gateway-conformance/actions/runs/5880695313/attempts/1#summary-15947714091, things standing out:

Groups like "Path Gateway" should ideally link to the "Path Gateway" spec. (I think Lidel had same comment.)
(Nice to have) Summarize results at the group level (e.g., "Path Gateway" is Green with X/X passing)
(Nice to have) Collapse the individual test results so one just sees the group and then can expand if interested? Maybe possible with https://stackoverflow.com/questions/28607382/is-it-possible-to-create-a-toggle-switch-in-markdown ? (Maybe this means having a table per test group?)
If can't have collapsed results, maybe consider different heading alignment? For example, "Path Gateway" could be center aligned rather than right aligned?
Is there a reason we have a "conformance-" prefix (e.g., "conformance-ipfs.runfission.com")?
(Idea) While prototyping, maybe also add in ipfs.io or dweb.link to show what it looks like when something is mostly green?

This is a drive by because I'm interested in the work. I likely won't be able to engage quickly here so don't block on me.

laurentsenta · 2023-09-07T15:45:38Z

Thanks for all the comments, these informed the new PRs!

@BigLep I took into account your comments, it'll be great to revisit these once we're done with the first iteration of the dashboard (wip)

Closing this, replaced by:

laurentsenta marked this pull request as draft August 4, 2023 11:23

laurentsenta force-pushed the feat/with-metadata branch from 972cf33 to 9b35422 Compare August 4, 2023 11:31

This comment was marked as outdated.

Sign in to view

laurentsenta force-pushed the feat/introduce-version branch from 57929ea to 0a9d68b Compare August 7, 2023 09:23

laurentsenta force-pushed the feat/with-metadata branch from 9b35422 to 44b0eaa Compare August 7, 2023 09:26

laurentsenta force-pushed the feat/introduce-version branch 3 times, most recently from b526505 to d1c8ff8 Compare August 15, 2023 14:57

laurentsenta added 2 commits August 15, 2023 17:26

feat: add version flag

919eda1

fix: t.Helper()

7b497e0

laurentsenta force-pushed the feat/with-metadata branch from 44b0eaa to a36fab8 Compare August 16, 2023 14:28

laurentsenta added 3 commits August 16, 2023 17:07

feat: introduce test group

5075cfc

feat: aggregate metadata

ac7f5cd

feat: aggregate into table uses metadata

a68b9de

laurentsenta force-pushed the feat/with-metadata branch from a36fab8 to e40a757 Compare August 16, 2023 15:08

laurentsenta added 2 commits August 16, 2023 17:17

feat: better grouping system

320966b

test: shorter wait for pinning

6303b3f

laurentsenta force-pushed the feat/with-metadata branch from e40a757 to 6303b3f Compare August 16, 2023 15:20

laurentsenta commented Aug 16, 2023

View reviewed changes

laurentsenta requested a review from galargh August 16, 2023 15:31

laurentsenta marked this pull request as ready for review August 16, 2023 15:31

laurentsenta requested review from lidel and hacdias August 16, 2023 15:31

galargh approved these changes Aug 17, 2023

View reviewed changes

This was referenced Aug 17, 2023

Generate Implementation Dashboard #123

Open

feat: output workflow url with test metadata #145

Merged

laurentsenta mentioned this pull request Aug 22, 2023

feat: generates parts of the dashboard async, using ipfs/gateway-conformance ipfs/public-gateway-checker#442

Open

13 tasks

laurentsenta closed this Sep 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add IPIP and TestGroup Metadatas #130

feat: add IPIP and TestGroup Metadatas #130

laurentsenta commented Aug 4, 2023 •

edited

Loading

This comment was marked as outdated.

laurentsenta Aug 16, 2023

laurentsenta Aug 16, 2023 •

edited

Loading

laurentsenta Aug 16, 2023

hacdias Aug 17, 2023 •

edited

Loading

lidel Aug 17, 2023 •

edited

Loading

laurentsenta commented Aug 16, 2023 •

edited

Loading

lidel commented Aug 17, 2023 •

edited

Loading

laurentsenta commented Aug 22, 2023

BigLep commented Aug 22, 2023

laurentsenta commented Sep 7, 2023

feat: add IPIP and TestGroup Metadatas #130

feat: add IPIP and TestGroup Metadatas #130

Conversation

laurentsenta commented Aug 4, 2023 • edited Loading

Todos

Follow-ups

This comment was marked as outdated.

laurentsenta Aug 16, 2023

Choose a reason for hiding this comment

laurentsenta Aug 16, 2023 • edited Loading

Choose a reason for hiding this comment

laurentsenta Aug 16, 2023

Choose a reason for hiding this comment

hacdias Aug 17, 2023 • edited Loading

Choose a reason for hiding this comment

lidel Aug 17, 2023 • edited Loading

Choose a reason for hiding this comment

laurentsenta commented Aug 16, 2023 • edited Loading

lidel commented Aug 17, 2023 • edited Loading

laurentsenta commented Aug 22, 2023

BigLep commented Aug 22, 2023

laurentsenta commented Sep 7, 2023

laurentsenta commented Aug 4, 2023 •

edited

Loading

laurentsenta Aug 16, 2023 •

edited

Loading

hacdias Aug 17, 2023 •

edited

Loading

lidel Aug 17, 2023 •

edited

Loading

laurentsenta commented Aug 16, 2023 •

edited

Loading

lidel commented Aug 17, 2023 •

edited

Loading