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(starlette): Remove invalid failed_request_status_code tests #3560

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

szokeasaurusrex
Copy link
Member

@szokeasaurusrex szokeasaurusrex commented Sep 24, 2024

The Starlette integration tests (as well as the FastAPI integration tests, which hit the same code path as the Starlette integration) include a test where the integrations' failed_request_status_codes parameter is set to [None]. However, since the parameter is typed as Optional[list[HttpStatusCodeRange]], where HttpStatusCodeRange = Union[int, Container[int]], passing [None] for this parameter should not be allowed, per the type hint. Thus, we should not test this input, since the behavior of passing [None] is not, and likely should not be, defined by the API.

Depends on #3562

Copy link

codecov bot commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.32%. Comparing base (5c6c778) to head (ccdbffb).
Report is 2 commits behind head on master.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3560      +/-   ##
==========================================
+ Coverage   84.30%   84.32%   +0.02%     
==========================================
  Files         133      133              
  Lines       13890    13890              
  Branches     2930     2930              
==========================================
+ Hits        11710    11713       +3     
  Misses       1440     1440              
+ Partials      740      737       -3     

see 3 files with indirect coverage changes

@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/remove-invalid-tests branch from 8402895 to 2afe13a Compare September 24, 2024 11:28
@szokeasaurusrex szokeasaurusrex changed the base branch from master to szokeasaurusrex/refactor-shared-parametrization September 24, 2024 11:28
…3560)

The Starlette integration tests (as well as the FastAPI integration tests, which hit the same code path as the Starlette integration) include a test where the integrations' `failed_request_status_codes` parameter is set to `[None]`. However, since the parameter is typed as `Optional[list[HttpStatusCodeRange]]`, where `HttpStatusCodeRange = Union[int, Container[int]]`, passing `[None]` for this parameter should not be allowed, per the type hint. Thus, we should not test this input, since the behavior of passing `[None]` is not, and should not be, defined by the API.
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/refactor-shared-parametrization branch from ee3ca40 to 5c6c778 Compare September 24, 2024 11:31
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/remove-invalid-tests branch from 2afe13a to ccdbffb Compare September 24, 2024 11:31
Base automatically changed from szokeasaurusrex/refactor-shared-parametrization to master September 24, 2024 12:08
@szokeasaurusrex szokeasaurusrex changed the base branch from master to potel-base September 24, 2024 12:09
@szokeasaurusrex szokeasaurusrex changed the base branch from potel-base to master September 24, 2024 12:09
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/remove-invalid-tests branch from 66453a2 to ccdbffb Compare September 24, 2024 12:10
@szokeasaurusrex szokeasaurusrex merged commit ccdbffb into master Sep 24, 2024
244 checks passed
@szokeasaurusrex szokeasaurusrex deleted the szokeasaurusrex/remove-invalid-tests branch September 24, 2024 12:20
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