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: add test 109 #33

Merged
merged 7 commits into from
Apr 5, 2023
Merged

feat: add test 109 #33

merged 7 commits into from
Apr 5, 2023

Conversation

laurentsenta
Copy link
Contributor

@laurentsenta laurentsenta commented Apr 5, 2023

Contributes to #26

}

if specs.SubdomainGateway.IsEnabled() {
Run(t, unwrapTests(t, tests).Build())
Copy link
Contributor Author

@laurentsenta laurentsenta Apr 5, 2023

Choose a reason for hiding this comment

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

@galargh I came up with a new pattern for testing with subdomains:
instead of explicitly calling a function on every test:

with(testGatewayWithManyProtocols(t,
"request for example.com/ipfs/{CIDv1} redirects to subdomain",
`
subdomains should not return payload directly,
but redirect to URL with proper origin isolation
`,
URL("%s/ipfs/%s/", gatewayURL, CIDv1),
Expect().

unwrapTests takes a bunch of tests relying on a subdomain, and unwrap them; it's more natural when you write the tests. Super easy to implement thanks to Builders being immutable.

It's composable also, so I'm this close 🤏 to moving the loop over multiple gateways into another unwrapper:

	gatewayURLs := []string{
		SubdomainGatewayURL,
		SubdomainLocalhostGatewayURL,
	}

=> unwrapTests(unwrapGateways(tests))

(You'd write the test once for local host gateway, and if subdomain parameter exists and is enabled, we'll clone every test cases)

Not perfect, but a step in a better direction I think, maybe that'll inspire some ideas on your side :)

"github.com/ipfs/go-cid"
"github.com/ipld/go-car/v2/blockstore"
Copy link
Contributor Author

@laurentsenta laurentsenta Apr 5, 2023

Choose a reason for hiding this comment

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

@galargh that's the fix for the crazy bug I shared yesterday,

  • I double-checked: init() is called only once per package as we expected.
  • When I renamed fixture.go into zfixture.go, the bug was fixed 🎉
  • I realized this had to be a collision between new boxo packages and the old ones.

Copy link
Contributor

Choose a reason for hiding this comment

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

🤯

@laurentsenta laurentsenta mentioned this pull request Apr 5, 2023
20 tasks
@galargh galargh merged commit 02f5802 into feat/add-t0122 Apr 5, 2023
@laurentsenta laurentsenta deleted the feat/add-t0109 branch April 5, 2023 12:04
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.

2 participants