fiber icon indicating copy to clipboard operation
fiber copied to clipboard

πŸ”₯ Feature: Add TestConfig to app.Test() for configurable testing

Open grivera64 opened this issue 1 year ago β€’ 2 comments

Description

This pull request modifies app.Test() in v3 to use a TestConfig{} struct to configure internal testing of app routes. This is due to the introduction of c.SendStreamWriter() in PR #3131, we need support for reading the response of a route after a certain timeout to mimic a client disconnection. It is important to keep this in account when streaming a response to the client, since you may need to do cleanup or rollback after a closed connection.

Fixes #3149

Changes introduced

  • [X] Documentation Update: Update old documentation of app.Test() in /docs/api/app.md
  • [X] Changelog/What's New: Add TestConfig struct to app.Test() for configurable testing
  • [ ] Migration Guide: If necessary, provide a guide or steps for users to migrate their existing code to accommodate these changes.
    • May be a good idea to include this change in the migration guide.
  • [X] Examples:
app := fiber.New()
app.Get("/", func(c Ctx) error {
  return c.Status(fiber.StatusOK).SendString("Hello World!")
}
resp, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/", nil), TestConfig{
  Timeout: -1, // No timeout
  ErrOnTimeout: false,
})

Type of change

  • [X] New feature (non-breaking change which adds functionality)
    • Adds the preservation of a timed-out response
    • Adds thread-safety to testConn
  • [X] Enhancement (improvement to existing features and functionality)
    • Add a TestConfig{} struct to perform the same functionality as before
    • Add unit tests for testConn
  • [X] Documentation update (changes to documentation)
    • Adds details about using the new version of the app.Test() method

Checklist

Before you submit your pull request, please make sure you meet these requirements:

  • [X] Followed the inspiration of the Express.js framework for new functionalities, making them similar in usage.
  • [X] Conducted a self-review of the code and provided comments for complex or critical parts.
  • [X] Updated the documentation in the /docs/ directory for Fiber's documentation.
  • [X] Added or updated unit tests to validate the effectiveness of the changes or new features.
  • [X] Ensured that new and existing unit tests pass locally with the changes.
  • [X] Verified that any new dependencies are essential and have been agreed upon by the maintainers/community.
  • [X] Aimed for optimal performance with minimal allocations in the new code.
  • [X] Provided benchmarks for the new code to analyze and improve upon.

grivera64 avatar Oct 09 '24 19:10 grivera64

Walkthrough

The changes introduce a new TestConfig struct to the fiber package, enhancing the app.Test method by allowing it to accept configuration settings for testing, including timeout duration and error handling for timeouts. This modification is reflected across various test files, where existing timeout parameters are replaced with instances of TestConfig. Additionally, several new tests and benchmarks have been added to improve coverage and performance measurement, particularly for context methods and connection handling.

Changes

File Change Summary
app.go Introduced TestConfig struct; modified Test method to accept TestConfig instead of timeout.
app_test.go Updated Test_App_Test to use TestConfig; added Test_App_Test_timeout_empty_response test case.
ctx_test.go Added benchmark tests and new test cases for context methods; enhanced error handling tests.
docs/api/app.md Updated Test method documentation to reflect changes to TestConfig.
helpers.go Modified testConn struct for thread safety; added locking mechanisms to Read, Write, and Close.
helpers_test.go Added tests for testConn operations; included benchmarks for new test functions.
middleware/compress/compress_test.go Updated tests to use TestConfig for timeout handling.
middleware/idempotency/idempotency_test.go Updated doReq function to use TestConfig in Test_Idempotency.
middleware/keyauth/keyauth_test.go Updated tests to utilize TestConfig for timeout settings.
middleware/logger/logger_test.go Updated logger tests to use TestConfig for request testing.
middleware/pprof/pprof_test.go Updated tests to utilize TestConfig for timeout settings.
middleware/proxy/proxy_test.go Updated tests to use TestConfig for request testing.
middleware/static/static_test.go Updated static file tests to use TestConfig for timeout and error handling.

Assessment against linked issues

Objective Addressed Explanation
Add TestConfig struct as an optional parameter to app.Test() for testing. βœ…
Allow app.Test() to specify error handling behavior for timeouts. βœ…
Ensure that existing functionality of app.Test() remains unchanged. βœ…

Possibly related PRs

  • #3172: The changes in this PR enhance the documentation for the SendFile function, which is directly related to the modifications made in the main PR regarding the Test_App_Test function in app_test.go, as both involve handling timeout configurations and response behaviors.
  • #3205: The updates to the API documentation and README include clarifications on the Test method, which is relevant to the changes made in the main PR that also involve the Test method's signature and its usage in tests.
  • #3206: The updates to the Context documentation include changes to method signatures and descriptions that are relevant to the modifications made in the main PR, particularly regarding the handling of context in tests and the introduction of new test cases.

Suggested labels

🧹 Updates

Suggested reviewers

  • sixcolors
  • gaby
  • efectn
  • ReneWerner87

Poem

🐰 In the meadow where the code does play,
A new TestConfig hops in today!
With timeouts set and errors in sight,
Testing is clearer, everything feels right.
So let’s cheer for the changes, oh what a delight! πŸŽ‰


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❀️ Share
πŸͺ§ Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Oct 09 '24 19:10 coderabbitai[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 82.85%. Comparing base (b6ecd63) to head (b6daa78). Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3161      +/-   ##
==========================================
+ Coverage   82.70%   82.85%   +0.15%     
==========================================
  Files         114      114              
  Lines       11163    11189      +26     
==========================================
+ Hits         9232     9271      +39     
+ Misses       1529     1520       -9     
+ Partials      402      398       -4     
Flag Coverage Ξ”
unittests 82.85% <100.00%> (+0.15%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


🚨 Try these New Features:

codecov[bot] avatar Oct 09 '24 19:10 codecov[bot]

I have added commits to address previous comments. I think the only failing check is due to an unrelated method, so this should be good from the checks perspective. Please let me know if you would like to make any more changes to the PR.

grivera64 avatar Nov 13 '24 05:11 grivera64

@grivera64 The benchmarks are failing related to the github-action we are using. I have reported the issue but there's no fix yet.

Related: https://github.com/benchmark-action/github-action-benchmark/issues/264

gaby avatar Nov 13 '24 05:11 gaby

@grivera64 Don't forget to update the "What's New" section, since this changes how using app.Test() works.

https://github.com/gofiber/fiber/blob/main/docs/whats_new.md

gaby avatar Nov 13 '24 05:11 gaby

@grivera64 The benchmarks are failing related to the github-action we are using. I have reported the issue but there's no fix yet.

Related: benchmark-action/github-action-benchmark#264

Interesting, would this affect the ability to work on Pull Requests?

grivera64 avatar Nov 13 '24 06:11 grivera64

@grivera64 Don't forget to update the "What's New" section, since this changes how using app.Test() works.

https://github.com/gofiber/fiber/blob/main/docs/whats_new.md

Thanks for the reminder @gaby , I've added an entry for app.Test() in the docs/whats_new.md file.

grivera64 avatar Nov 13 '24 06:11 grivera64

Thanks for the feedback @ReneWerner87 ! I've updated the code and documentation per the requests. Please let me know if we should make any more changes or additions to the PR.

grivera64 avatar Nov 19 '24 23:11 grivera64

Thanks for the approval @efectn !

For some reason, the change request from a few days ago hasn't been cleared after pushing changes and resolving the comment. @ReneWerner87 Please let me know if I missed something from the change request that could be causing this issue.

Thanks again!

grivera64 avatar Nov 22 '24 00:11 grivera64