Skip to content

Improved rate limiting tests #45541

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

Merged
merged 2 commits into from
Dec 12, 2022

Conversation

MadL1me
Copy link
Contributor

@MadL1me MadL1me commented Dec 11, 2022

Improved rate limiting middleware tests

Added rate limiting middleware tests improvements, based on #43298 issue

Description

  • Unit tests now using RateLimiterEndpointConventionBuilderExtensions where applicable
  • Added Assert-Act-Arrange comments for more clarification
  • Extracted method for rate limit middleware initialization
  • Extracted build classes in another file

@ghost ghost added area-runtime community-contribution Indicates that the PR has been added by a community member labels Dec 11, 2022
@ghost
Copy link

ghost commented Dec 11, 2022

Thanks for your PR, @MadL1me. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@dnfadmin
Copy link

dnfadmin commented Dec 11, 2022

CLA assistant check
All CLA requirements met.

@MadL1me MadL1me changed the title #43298 - Improved rate limiting tests Improved rate limiting tests Dec 11, 2022
@MadL1me MadL1me force-pushed the MadL1me/rate-limit-test-improvements branch from 14302d3 to dd3ef30 Compare December 12, 2022 11:55
@Tratcher Tratcher requested a review from wtgodbe December 12, 2022 17:09

namespace Microsoft.AspNetCore.RateLimiting;

internal class TestEndpointBuilder : EndpointBuilder
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename this file to TestEndpointBuilder.cs

Copy link
Member

@wtgodbe wtgodbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the 2 small comments, looks great! Thanks for the change. Will approve & merge once those 2 changes have been made.

@wtgodbe
Copy link
Member

wtgodbe commented Dec 12, 2022

Looks great! Will merge once CI passes.

@wtgodbe wtgodbe enabled auto-merge (squash) December 12, 2022 20:15
@MadL1me
Copy link
Contributor Author

MadL1me commented Dec 12, 2022

@wtgodbe Done!

Fixed file naming and removed project from solution filter.

I would be happy to work on other "help wanted issues". Currently found 2 more interesting issues related to RateLimiter as well.

@wtgodbe wtgodbe merged commit 88ba76b into dotnet:main Dec 12, 2022
@ghost ghost added this to the 8.0-preview1 milestone Dec 12, 2022
@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants