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

oonirun(v2.go): document and review implementation and tests #2716

Closed
bassosimone opened this issue Apr 24, 2024 · 1 comment · Fixed by ooni/probe-cli#1565
Closed

oonirun(v2.go): document and review implementation and tests #2716

bassosimone opened this issue Apr 24, 2024 · 1 comment · Fixed by ooni/probe-cli#1565
Assignees

Comments

@bassosimone
Copy link
Member

I am about to replace httpx with httpclientx (as implemented in ooni/probe-cli#1560) for the ./internal/oonirunv2 package. Before doing this, I would like to review the implementation and the tests to make sure we have adequate testing and that it's clear what we're doing and why we're doing it. The target files are v2.go and v2_test.go.

This work is part of more comprehensive work described by #2700.

@bassosimone
Copy link
Member Author

It seems we're already covering these cases with tests, while using a real localhost HTTP server:

  • common case where we fetch and execute a descriptor
  • case where we fetch it but cannot update the cache
  • case where we fetch it but there's no permission to automerge changes
  • case where the server returns a literal "null"
  • case where the server returns a descriptor with an empty test name

I do not think the job of the oonirun package would be to test its underlying HTTP library. That sad, because I am changing the underlying library, and for robustness, it won't certainly hurt to add:

  • case where the connection is RST when attempting to fetch a descriptor
  • case where the JSON does not parse

Since adding these new test cases would be quick, I think it makes sense since it adds a bit of robustness.

bassosimone added a commit to ooni/probe-cli that referenced this issue Apr 24, 2024
@bassosimone bassosimone changed the title oonirunv2: document and review implementation and tests oonirun(v2.go): document and review implementation and tests Apr 24, 2024
bassosimone added a commit to ooni/probe-cli that referenced this issue Apr 24, 2024
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 a pull request may close this issue.

1 participant