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

mock HTTP requests in the test suite #4621

Merged
merged 3 commits into from
May 10, 2024
Merged

Conversation

danc86
Copy link
Contributor

@danc86 danc86 commented Apr 1, 2024

Description

I'm working on packaging fontbakery in nixpkgs, which requires that the test suite run without network access. I found a few tests which intentionally make network calls, but in most of them it seems desirable to mock out requests to live services. This will make them faster and more reproducible, and lets them run in an environment like nixpkgs where they are sandboxed from the network.

Checklist

  • update CHANGELOG.md
  • wait for the tests to pass
  • request a review

@danc86 danc86 mentioned this pull request Apr 1, 2024
3 tasks
@danc86
Copy link
Contributor Author

danc86 commented Apr 1, 2024

With these changes, the only remaining test case in the regression test suite which makes a network request is test_check_cjk_vertical_metrics_regressions().

If you like this approach of using requests-mock, I could refactor the remote_styles condition to also use requests to download the fonts zip file, and then update test_check_cjk_vertical_metrics_regressions() to return a mock zip file in the tests.

That would allow the complete test suite to run without making any network requests.

danc86 added 3 commits May 9, 2024 17:14
This seems to be equivalent, and it makes it easier to mock the
necessary HTTP requests in the test cases for the vertical_metrics and
cjk_vertical_metrics conditions. We only need to mock the metadata
lookup, rather than a complete zip file that is never used.
This makes it easier to mock out the request to Github in the tests.
This lets the test suite work in environments where network access is
restricted. It also lets us easily test for HTTP errors like timeouts.
@felipesanches felipesanches merged commit 95d1c4f into fonttools:main May 10, 2024
55 checks passed
@felipesanches
Copy link
Collaborator

Hey, @danc86, it took me a long time to do it, but here it is: reviewed and merged! ;-)

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