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

Only call super() during MockHttp if required #2033

Merged
merged 6 commits into from
Mar 2, 2025

Conversation

s-t-e-v-e-n-k
Copy link
Contributor

@s-t-e-v-e-n-k s-t-e-v-e-n-k commented Sep 3, 2024

Only instantiate superclasses of MockHttp if we are provided keyword arguments

Description

With pytest 8.2 and above, any class that contains tests is collected, which means they are instantiated, which MockHttp's superclasses do not accept, since they require keyword arguments. To work around this, only call the superclass's init method if we are passed kwargs.

Status

Done, ready for review

Checklist (tick everything that applies)

  • Code linting (required, can be done after the PR checks)
  • Documentation (as code comments)
  • Tests
  • ICLA (required for bigger changes)

With pytest 8.2 and above, any class that contains classes is collected,
which means they are instantiated, which MockHttp's superclasses do not
accept, since they require keyword arguments. To work around this, only
call the superclass's __init__ method if we are passed kwargs.
@Kami Kami added this to the v3.9.0 milestone Mar 2, 2025
@Kami Kami added the tests label Mar 2, 2025
@Kami
Copy link
Member

Kami commented Mar 2, 2025

Thanks for the contribution.

I've intentionally help back at upgrading pytest since we rely on test collection logic which has changed in new version of pytest so various tests are not being collected correctly.

I will test and verify this change locally with latest version of pytest.

@Kami
Copy link
Member

Kami commented Mar 2, 2025

@s-t-e-v-e-n-k I've tested it locally and it indeed appears to be working fine.

Before:

==================================== 8156 passed, 13 skipped, 33 warnings in 21.37s =====================================
========================================== 2 passed, 8169 deselected in 1.18s ===========================================

After (with pytest > 8.2):

==================================== 8157 passed, 13 skipped, 33 warnings in 22.42s =====================================
========================================== 2 passed, 8170 deselected in 1.18s ===========================================

I will merge it into trunk shortly. Thanks.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.40%. Comparing base (161e446) to head (fed3944).

Additional details and impacted files
@@            Coverage Diff             @@
##            trunk    #2033      +/-   ##
==========================================
- Coverage   83.40%   83.40%   -0.00%     
==========================================
  Files         353      353              
  Lines       81684    81685       +1     
  Branches     8631     8632       +1     
==========================================
  Hits        68124    68124              
  Misses      10738    10738              
- Partials     2822     2823       +1     
Files with missing lines Coverage Δ
libcloud/test/__init__.py 86.78% <100.00%> (-0.72%) ⬇️

@Kami Kami merged commit 1117987 into apache:trunk Mar 2, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants