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

testing: allow to instantiate an empty AsyncTestCase #3374

Closed
wants to merge 1 commit into from

Conversation

bluetech
Copy link
Contributor

unittest.TestCase has a feature where it allows instantiating MyTestClass() with the default method name runTest even if a runTest method doesn't actually exist. This is documented in TestCase's docs under "Changed in version 3.2" (link).

Since version 8.2, pytest relies on this, and started breaking on Tornado's AsyncTestCase, see pytest-dev/pytest#12263.

Change AsyncTestCase to allow empty instatiation, by matching the upstream code.

`unittest.TestCase` has a feature where it allows instantiating
`MyTestClass()` with the default method name `runTest` even if a
`runTest` method doesn't actually exist. This is documented in
`TestCase`'s docs under "Changed in version 3.2"[0].

Since version 8.2, pytest relies on this, and started breaking on
Tornado's `AsyncTestCase`[1].

Change `AsyncTestCase` to allow empty instatiation, by matching the
upstream code.

[0] https://docs.python.org/3/library/unittest.html#unittest.TestCase
[1] pytest-dev/pytest#12263
MichaIng added a commit to motioneye-project/motioneye that referenced this pull request May 11, 2024
The motion package installs a motion systemd service, which can block camera devices and hence conflict with the motionEye-controlled motion processes.

We hence need to assure that this service is stopped and disabled, before we can start and enable out motioneye.service.

Additionally, Tornado python tests are ignored, until a related fix has been merged: tornadoweb/tornado#3374

Signed-off-by: MichaIng <[email protected]>
@arossert
Copy link

@bluetech Any estimation when this will be merged and released?

@bdarnell
Copy link
Member

I want to look at this next week and understand what's going on. Catching an exception and disabling the _TestMethodWrapper for pytest users doesn't seem like the right solution and the "new in 3.2" docs look unrelated (it sounds like this is when the methodName parameter got its default value but has no bearing on whether the named method must exist).

@bluetech
Copy link
Contributor Author

bluetech commented Jun 3, 2024

Catching an exception and disabling the _TestMethodWrapper for pytest users doesn't seem like the right solution

Let me try to clarify: pytest instantiates the test class in two situations:

  1. Once per test class, to collect fixtures and tests and such.
  2. One per test, to use as the test's self.

This PR is meant to support the first case. It is not disabling the _TestMethodWrapper in the second case, when it matters, only in the first, when no method is being run.

the "new in 3.2" docs look unrelated (it sounds like this is when the methodName parameter got its default value but has no bearing on whether the named method must exist).

Hmm yes that note is ambiguous but let me try to clarify it. It says: "Changed in version 3.2: TestCase can be instantiated successfully without providing a methodName". The situation before was that there was a methodName="runTest" default, but the runTest method was actually expected to exist, so in effect TestCase could not be instantiated successfully without providing a methodName. What the change did was to take "runTest" as a sentinel value to allow to instantiate even if a runTest method is not defined. Here is the original upstream commit: python/cpython@32e1d83

@bdarnell
Copy link
Member

bdarnell commented Jun 3, 2024

Yes, I was just researching this and was pulling on the history. I see a better fix for this by moving the check in _TestCaseWrapper to a different location instead of __init__ time. New PR coming soon.

bdarnell added a commit to bdarnell/tornado that referenced this pull request Jun 3, 2024
Overriding _callTestMethod (which was introduced in python 3.8) is a
less hacky way to detect tests that fail to use ``@gen_test`` where
needed. It's not documented, but since Python 3.11 has introduced a
similar check to the standard library we'll be able to remove it in the
near future.

The major impetus for this change is an incompatibility with
Pytest 8.2, which has made a change that tries to instantiate test
classes at discovery time without an existing method name.

Fixes tornadoweb#3375
Closes tornadoweb#3374
@bdarnell
Copy link
Member

bdarnell commented Jun 3, 2024

Replacement PR is #3382

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.

3 participants