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: compress middleware custom error message #2079

Closed
wants to merge 2 commits into from

Conversation

ariskemper
Copy link
Contributor

@ariskemper ariskemper commented Jan 24, 2024

Author should do the followings, if applicable

  • Add tests
  • Run tests
  • yarn denoify to generate files for Deno

@ariskemper
Copy link
Contributor Author

@usualoma @yusukebe extra tests for not found error with compress middleware

@usualoma
Copy link
Member

@ariskemper

Thanks for creating the PR.

I like the idea of more testing, but on the other hand I think it is also true that more test code is also more maintenance cost. I believe we need a test code for our needs.

I think the file src/middleware/compress/index.test.ts should contain tests related to the middleware of compress. The “matched route that includes only one middleware breaks applying order “ (#2080) is a bug of hono's own, not of the middleware.
Also, the middleware of compress does not contain any code related to 404.

Given that, I see no need for this PR test code in src/middleware/compress/index.test.ts.

Do you think that "in the future, regressions that cannot be detected by the test code of hono itself can be detected by this test code?"

@ariskemper
Copy link
Contributor Author

ariskemper commented Jan 25, 2024

@usualoma i agree with you more tests is more maintenance, so lets keep it as you mentioned, but please check honojs/node-server#130

@ariskemper ariskemper closed this Jan 25, 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 this pull request may close these issues.

2 participants