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 for HTTP spec conformance during CI #3083

Open
kyri-petrou opened this issue Sep 3, 2024 · 8 comments · May be fixed by #3169
Open

Test for HTTP spec conformance during CI #3083

kyri-petrou opened this issue Sep 3, 2024 · 8 comments · May be fixed by #3169
Labels
💎 Bounty maintenance Chore or Maintenance tasks

Comments

@kyri-petrou
Copy link
Collaborator

I think this is a bit of an ambitious request, but hopefully it's something that we can achieve.

The HTTP spec contains a lot of "gotchas" and unless a contributor knows the HTTP spec inside-out (which is almost impossible), we're bound to make a change at some point that violates the spec. A good example of this is #3080, where a previous fix to an issue that allowed users to provide an invalid content-length (which is against the spec) seems to have broken HEAD requests, where the content-length should match the length of the content as if the request is a GET request.

In order to guard against this, we need an HTTP Spec conformance test suite that runs during CI. A potentially good starting point is this repo, but there are probably other / better tools for it (haven't looked too extensively). I'm sure that adding such a suite will very likely unearth violations, so we should probably do it in 2-3 steps:

  1. Add a CI workflow that tests a server for HTTP spec conformance.
  2. Fix any failing checks that are required by the spec.
  3. Fix as many recommendations as possible.

@jdegoes @987Nabil what are your thoughts on having this? Is this something that's too ambitious or do you think it might be doable?

@kyri-petrou kyri-petrou added the maintenance Chore or Maintenance tasks label Sep 3, 2024
@987Nabil
Copy link
Contributor

987Nabil commented Sep 3, 2024

I think it is a good idea. Maybe we don't need to add all checks at the beginning. We could establish the process first and add things we fixed our think are very important and imperatively improve the checks

@jdegoes
Copy link
Member

jdegoes commented Sep 18, 2024

/bounty $2500

Copy link

algora-pbc bot commented Sep 18, 2024

💎 $2,500 bounty • ZIO

Steps to solve:

  1. Start working: Comment /attempt #3083 with your implementation plan
  2. Submit work: Create a pull request including /claim #3083 in the PR body to claim the bounty
  3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

Thank you for contributing to zio/zio-http!

Add a bountyShare on socials

Attempt Started (GMT+0) Solution
🟡 @Saturn225 Sep 18, 2024, 2:20:27 PM #3169

@Saturn225
Copy link

Saturn225 commented Sep 18, 2024

/attempt #3083

@digital-phoenix
Copy link

@jdegoes could you further define the scope of this bounty? Is it just to add an HTTP Spec conformance test suite that runs during CI? Or does this bounty also require all failing tests to be fixed after the test suite is implemented?

@ayewo
Copy link

ayewo commented Sep 25, 2024

@jdegoes
Another scope-related question: since the conformance test suite will mostly be run in a CI environment, is it mandatory to write it in Scala?

The ZIO org has about 8 non-Scala repos so was wondering if you don't mind at all if a fork of the Python repo @kyri-petrou linked to is used as the basis for this task?

Copy link

algora-pbc bot commented Sep 25, 2024

@SankeerthKoraboina: Reminder that in 7 days the bounty will become up for grabs, so please submit a pull request before then 🙏

@Saturn225 Saturn225 linked a pull request Sep 26, 2024 that will close this issue
Copy link

algora-pbc bot commented Sep 26, 2024

💡 @Saturn225 submitted a pull request that claims the bounty. You can visit your bounty board to reward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💎 Bounty maintenance Chore or Maintenance tasks
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants