Skip to content

test(starlette): Remove invalid failed_request_status_code tests #3560

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants