-
Notifications
You must be signed in to change notification settings - Fork 14
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
Conversation
} | ||
|
||
if specs.SubdomainGateway.IsEnabled() { | ||
Run(t, unwrapTests(t, tests).Build()) |
There was a problem hiding this comment.
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:
gateway-conformance/tests/t0114_gateway_subdomains_test.go
Lines 49 to 56 in 023888a
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" |
There was a problem hiding this comment.
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
intozfixture.go
, the bug was fixed 🎉 - I realized this had to be a collision between new boxo packages and the old ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤯
Contributes to #26