Skip to content
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

test(starlette): Refactor shared test parametrization #3562

Merged

Conversation

szokeasaurusrex
Copy link
Member

We use the same parametrization for testing FastAPI's and Starlette's failed_request_status_codes because the FastApiIntegration's constructor is the same as StarletteIntegration's constructor (the former is a subclass of the latter).

Here, we refactor the test cases to define the parametrization once, then use it in both tests. This change will make some future changes simpler, since we only need to change the parameters in one place to affect the test for both frameworks.

Copy link

codecov bot commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.30%. Comparing base (7e4992a) to head (5c6c778).
Report is 3 commits behind head on master.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3562      +/-   ##
==========================================
- Coverage   84.31%   84.30%   -0.01%     
==========================================
  Files         133      133              
  Lines       13890    13890              
  Branches     2930     2930              
==========================================
- Hits        11711    11710       -1     
  Misses       1440     1440              
- Partials      739      740       +1     

see 1 file with indirect coverage changes

We use the same parametrization for testing FastAPI's and Starlette's `failed_request_status_codes` because the `FastApiIntegration`'s constructor is the same as `StarletteIntegration`'s constructor (the former is a subclass of the latter).

Here, we refactor the test cases to define the parametrization once, then use it in both tests. This change will make some future changes simpler, since we only need to change the parameters in one place to affect the test for both frameworks.
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/refactor-shared-parametrization branch from ee3ca40 to 5c6c778 Compare September 24, 2024 11:31
@szokeasaurusrex szokeasaurusrex merged commit 5c6c778 into master Sep 24, 2024
125 checks passed
@szokeasaurusrex szokeasaurusrex deleted the szokeasaurusrex/refactor-shared-parametrization branch September 24, 2024 12:08
# 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