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

test(gateway): use deterministic CAR fixtures #9657

Merged
merged 12 commits into from
Feb 28, 2023

Conversation

laurentsenta
Copy link
Contributor

@laurentsenta laurentsenta commented Feb 20, 2023

Contributes to #9654

  • t0109-gateway-web-_redirects.sh
    • no change required
  • t0111-gateway-writeable.sh
    • no change: we generate random data here. writable gateways will be covered later.
  • t0112-gateway-cors.sh
    • no change required
  • t0113-gateway-symlink.sh
  • t0114-gateway-subdomains.sh
    • ipfs, no change for ipns yet
    • ipns (not doing ipnfs yet)
  • t0115-gateway-dir-listing.sh
  • t0116-gateway-cache.sh
  • t0117-gateway-block.sh
  • t0118-gateway-car.sh
  • t0122-gateway-tar.sh
  • t0123-gateway-json-cbor.sh
  • t0124-gateway-ipns-record.sh (not doing ipns yet)
  • t0400-api-no-gateway.sh

@laurentsenta laurentsenta requested a review from lidel as a code owner February 20, 2023 15:45
@laurentsenta laurentsenta force-pushed the test/sharness-with-car-fixtures branch from b1492fb to a4086e6 Compare February 21, 2023 10:32
@laurentsenta laurentsenta force-pushed the test/sharness-with-car-fixtures branch 2 times, most recently from ff609da to 89e62af Compare February 23, 2023 10:43
@laurentsenta laurentsenta requested a review from hacdias February 23, 2023 10:50
@laurentsenta laurentsenta force-pushed the test/sharness-with-car-fixtures branch 2 times, most recently from c0a3bdb to 8e636e6 Compare February 24, 2023 11:21
@laurentsenta laurentsenta changed the title test: sharness with deterministic car fixtures test: use deterministic fixtures in gateway sharness tests Feb 24, 2023
@laurentsenta laurentsenta changed the title test: use deterministic fixtures in gateway sharness tests test: use deterministic fixtures during gateway testing Feb 24, 2023
@laurentsenta laurentsenta changed the title test: use deterministic fixtures during gateway testing test: use deterministic fixtures during sharness gateway testing Feb 24, 2023
@laurentsenta laurentsenta force-pushed the test/sharness-with-car-fixtures branch from 1478aa1 to 1ed8b35 Compare February 24, 2023 12:51
@laurentsenta laurentsenta requested a review from galargh February 24, 2023 12:51
Copy link
Member

@hacdias hacdias left a comment

Choose a reason for hiding this comment

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

Overall looks good to me. This is very nice 😃 I left some inline comments.

@laurentsenta laurentsenta force-pushed the test/sharness-with-car-fixtures branch from f4cd620 to 92a068a Compare February 27, 2023 11:17
@laurentsenta
Copy link
Contributor Author

Thanks for the review @hacdias, I patched your comments,
and I removed the IPNS change for now (it started breaking for no apparent reason). I'll add this in a second PR.

go test is failing at the moment but since we haven't touched code other than sharness shell scripts, I guess this is unrelated flakiness.

@laurentsenta laurentsenta requested a review from hacdias February 27, 2023 11:56
Copy link
Member

@hacdias hacdias left a comment

Choose a reason for hiding this comment

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

I re-run the go test in GitHub and it's fine now. There are a few flaky tests and they seem to fail more often in GitHub Actions than Circle CI actually.

@laurentsenta laurentsenta changed the title test: use deterministic fixtures during sharness gateway testing test: use deterministic ipfs fixtures during sharness gateway testing Feb 27, 2023
@lidel lidel changed the title test: use deterministic ipfs fixtures during sharness gateway testing test(gateway): use deterministic CAR fixtures Feb 28, 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 @laurentsenta! ❤️

This work is not only unblocking work on gateway conformance test suite that is not tied to Kubo, but ALSO reduces the surface work for switching ipfs add to produce CIDv1 by default.

@lidel lidel merged commit cf9276d into ipfs:master Feb 28, 2023
@galargh
Copy link
Contributor

galargh commented Mar 1, 2023

I re-run the go test in GitHub and it's fine now. There are a few flaky tests and they seem to fail more often in GitHub Actions than Circle CI actually.

I checked success ratios of go-test in the last 24 hours. It's 29% for CircleCI and 29.4% for GitHub Actions. CircleCI can't do fractions so it seems to be the same thing. I didn't go further than 24 hours because we only started collecting GitHub Actions data 3 days ago and CircleCI only offers insights in 1, 7, 30, 60, 90 day intervals.

interop is the only test that seems to be doing worse on GHA. 92% vs 73.3%. Luckily, it's only on the surface. CircleCI cheats here a bit because it doesn't count cases where build failed and interop couldn't even start; like this one https://github.com/ipfs/kubo/pull/9677/checks?check_run_id=11653777666.

Overall, it seems we're pretty much on track to be able to disable CircleCI. I'll dig into the data more before the final cleanup PR but there's nothing worrying standing out.

Here's the WIP GHA monitoring dashboard if you want a peek: https://protocollabs.grafana.net/d/gieYOhbVk/github-actions-ipfs-kubo?orgId=1&from=now-3d&to=now (cc @BigLep since I think you might be interested ;)

@hacdias
Copy link
Member

hacdias commented Mar 1, 2023

@galargh interesting. My "it seems" is just based on my observations on my PRs specifically, so I might just be unlucky :P Thanks for the link!

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.

4 participants