refactor(testing): deprecate testtools support in TestCase #2405
Conversation
Remove conditional dependency on testtools in TestCase to simplify maintenance and prepare for Falcon 5.0 where testtools will no longer be supported. Instead, unittest is now the default, and users are encouraged to use pytest for testing Falcon applications. This change emits a deprecation warning if testtools is still enabled via environment variables. Closes falconry#2156
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2405 +/- ##
===========================================
- Coverage 100.00% 98.15% -1.85%
===========================================
Files 64 64
Lines 7726 7742 +16
Branches 1071 1073 +2
===========================================
- Hits 7726 7599 -127
- Misses 0 123 +123
- Partials 0 20 +20 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thanks once again for this PR. 👍
It seems that it needs work before we can merge it, though:
- A newsfragment is missing.
- Test coverage for new code is missing.
- The environment variable could be called
FALCON_TEST_CASE, orFALCON_BASE_TEST_CASE. The value could be an import path with or without the.TestCaseor::TestCasesuffix, e.g.,FALCON_BASE_TEST_CASE=testtools.TestCase. - We shouldn't emit a deprecation warning just because
testtoolscan be found in the current environment, but only when aTestCaseis initialized with the base fromtesttoolsautomagically. If provided via the envvar,testtoolsshould continue to emit no warning. - I don't think we should write that we recommend
pytestoverunittest; although personally I could maybe sign under that recommendation,unittestcontinues to be very popular too. Moreover, you can usepytestto rununittest.TestCases, allowing even for a hybrid approach.
still missing test coverage
There was a problem hiding this comment.
Please do not remove important notes that were not added by you!
It seems that the updated changes are not really what was requested, could you take a look again? Thanks!
Moreover, code coverage does not reach 100%. You can run tox locally to check coverage before pushing changes for a public review; this might be more efficient in terms of the number of unnecessary iterations in the review process.
|
|
||
| import falcon | ||
| import falcon.request | ||
|
|
||
| # TODO hoist for backwards compat. Remove in falcon 4. |
There was a problem hiding this comment.
Please don't remove this note!
We missed it in Falcon 4.0 it seems, so it needs to be updated to say Falcon 5.0, if you want to touch it.
falcon/testing/test_case.py
Outdated
| import testtools | ||
|
|
||
| warnings.warn( | ||
| 'Support for testtools is deprecated and will be removed in Falcon 5.0.', |
There was a problem hiding this comment.
As requested before, please do not warn just because the package is importable!
Only warn when a TestCase is actually initialized.
falcon/testing/test_case.py
Outdated
| from falcon.testing.client import Result # NOQA | ||
| from falcon.testing.client import TestClient | ||
|
|
||
| base_case = os.environ.get('FALCON_BASE_TEST_CASE') | ||
|
|
||
| if base_case and 'testtools.TestCase' in base_case: |
There was a problem hiding this comment.
We need to actually import the specified module, not just support testtools.TestCase.
It can be something.else.TestCase too.
| @@ -0,0 +1,3 @@ | |||
| - **Deprecation**: Support for `testtools` in `falcon.testing` is now deprecated and will be removed in Falcon 5.0. | |||
There was a problem hiding this comment.
Don't name the newsfragment .breakingchange., because we are not making any actual breaking changes until 5.0.
Since we are adding a new way to configure the base test case, I suggest marketing this as new feature.
| """ | ||
|
|
||
| # NOTE(vytas): Here we have to restore __test__ to allow collecting tests! |
Summary of Changes
Remove conditional dependency on testtools in TestCase to simplify maintenance and prepare for Falcon 5.0 where testtools will no longer be supported. Instead, unittest is now the default, and users are encouraged to use pytest for testing Falcon applications. This change emits a deprecation warning if testtools is still enabled via environment variables.
Related Issues
Closes #2156