GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — `TestResult.stopTest` called when test is skipped, despite `TestResult.startTest` not being called · Issue #113267 · python/cpython

TestResult.stopTest called when test is skipped, despite TestResult.startTest not being called #113267

New issue
Copy link
New issue
Copy link
Closed
Closed
TestResult.stopTest called when test is skipped, despite TestResult.startTest not being called#113267
Copy link
Labels
3.12only security fixesonly security fixes3.13only security fixesonly security fixesstdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

Description

@yvan-the-barbarian
yvan-the-barbarian
opened on Dec 19, 2023
Issue body actions

Bug report

Bug description:

Following commit 551aa6ab9419109a80ad53900ad930e9b7f2e40d a bug seems to have been introduced in the TestCase.run method.

Because the result.startTest(self) call was moved inside the try/finally block after the check to see if the test is skipped, the result.stopTest(self) call will be made even if the test is skipped and startTest is never called.

While this does not cause issues with the standard TestResult or TextTestResult classes, it can cause issues with other classes which subclass those and expect every call to stopTest to be preceded by a call from startTest.

Most notably for me, the unittest-xml-reporting package, whose stopTest method uses data initialized in startTest.

Note: Further thinking about this problem, it seems like changing the flow of methods like this was ill thought solution to the problem in the first place, as this might be an unexpected change of behaviour for different libraries. I'm still leaving what I think could be the solution in case I'm wrong on that.

It looks like the fix would seem to be moving the call to stopTest in yet another try/finally block right after the call to startTest, emulating the same behaviour as previous code while keeping the call to startTest after the skip check. The call to stopTestRun would need to remain where it is as startTestRun has already been called at this point.

So using the code as found in that commit, something like:

    def run(self, result=None):
        if result is None:
            result = self.defaultTestResult()
            startTestRun = getattr(result, 'startTestRun', None)
            stopTestRun = getattr(result, 'stopTestRun', None)
            if startTestRun is not None:
                startTestRun()
        else:
            stopTestRun = None

        try:
            testMethod = getattr(self, self._testMethodName)
            if (getattr(self.__class__, "__unittest_skip__", False) or
                getattr(testMethod, "__unittest_skip__", False)):
                # If the class or method was skipped.
                skip_why = (getattr(self.__class__, '__unittest_skip_why__', '')
                            or getattr(testMethod, '__unittest_skip_why__', ''))
                _addSkip(result, self, skip_why)
                return result

            # Increase the number of tests only if it hasn't been skipped
            result.startTest(self)
            try:
                expecting_failure = (
                    getattr(self, "__unittest_expecting_failure__", False) or
                    getattr(testMethod, "__unittest_expecting_failure__", False)
                )
                outcome = _Outcome(result)
                start_time = time.perf_counter()
                try:
                    self._outcome = outcome

                    with outcome.testPartExecutor(self):
                        self._callSetUp()
                    if outcome.success:
                        outcome.expecting_failure = expecting_failure
                        with outcome.testPartExecutor(self):
                            self._callTestMethod(testMethod)
                        outcome.expecting_failure = False
                        with outcome.testPartExecutor(self):
                            self._callTearDown()
                    self.doCleanups()
                    self._addDuration(result, (time.perf_counter() - start_time))

                    if outcome.success:
                        if expecting_failure:
                            if outcome.expectedFailure:
                                self._addExpectedFailure(result, outcome.expectedFailure)
                            else:
                                self._addUnexpectedSuccess(result)
                        else:
                            result.addSuccess(self)
                    return result
                finally:
                    # explicitly break reference cycle:
                    # outcome.expectedFailure -> frame -> outcome -> outcome.expectedFailure
                    outcome.expectedFailure = None
                    outcome = None

                    # clear the outcome, no more needed
                    self._outcome = None
                    
            finally:
                result.stopTest(self)
            
        finally:
            if stopTestRun is not None:
                stopTestRun()

CPython versions tested on:

3.12

Operating systems tested on:

Linux, Windows

Linked PRs

  • gh-113267: Revert "gh-106584: Fix exit code for unittest in Python 3.12 (#106588)" #114470
  • [3.12] gh-113267: Revert "gh-106584: Fix exit code for unittest in Python 3.12 (GH-106588)" (GH-114470) #114994
Reacted by Gilles Leblanc and Zied

Activity

  1. yvan-the-barbarian
    added
    type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
    on Dec 19, 2023
  2. AlexWaygood
    added
    stdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directory
    on Dec 19, 2023
  3. yvan-the-barbarian
    changed the title [-]`TestResult.stopTest` called when test is skipped, despite `TestResult.startTest`[/-] [+]`TestResult.stopTest` called when test is skipped, despite `TestResult.startTest` not being called[/+] on Dec 19, 2023
  4. yvan-the-barbarian commented on Jan 4, 2024

    @yvan-the-barbarian
    yvan-the-barbarian
    on Jan 4, 2024
    Author
    More actions

    I am now also seeing failures in PyCharm's unit testing because of this issue.

    This is starting to be a huge issue.

  5. gpshead
    added
    3.12only security fixesonly security fixes
    on Jan 23, 2024
  6. serhiy-storchaka commented on Jan 23, 2024

    @serhiy-storchaka
    serhiy-storchaka
    on Jan 23, 2024
    Member
    More actions

    I think that it was a wrong change, and not only because it makes startTest() and stopTest() calls unbalanced, but because it did not handle all cases and changed the behavior in wrong direction. #113661 was the proper fix of the original problem.

  7. serhiy-storchaka
    added
    3.13only security fixesonly security fixes
    on Jan 23, 2024
  8. serhiy-storchaka
    added a commit that references this issue on Feb 4, 2024

    gh-113267: Revert "gh-106584: Fix exit code for unittest in Python 3.…

    ecabff9
  9. miss-islington
    added a commit that references this issue on Feb 4, 2024

    pythongh-113267: Revert "pythongh-106584: Fix exit code for unittest …

    b79c37f
  10. serhiy-storchaka
    added a commit that references this issue on Feb 4, 2024

    [3.12] gh-113267: Revert "gh-106584: Fix exit code for unittest in Py…

    0a73763
  11. sodul commented on Feb 7, 2024

    @sodul
    sodul
    on Feb 7, 2024
    More actions

    Thanks, the changes for 3.12.2 seem to fix CleanCut/green#277

  12. serhiy-storchaka
    closed this as completedon Feb 7, 2024
  13. aisk
    added a commit that references this issue on Feb 11, 2024

    pythongh-113267: Revert "pythongh-106584: Fix exit code for unittest …

    9d44784
  14. fsc-eriker
    added a commit that references this issue on Feb 14, 2024

    pythongh-113267: Revert "pythongh-106584: Fix exit code for unittest …

    39378cc
  15. wesleybl
    added a commit that references this issue on Feb 27, 2024

    Use last version of Python 3.12 in tests

    199a22e
  16. davisagli
    added a commit that references this issue on Feb 28, 2024

    Use last version of Python 3.12 in tests (#1756)

    1771c9c
  17. mister-roboto
    added 2 commits that reference this issue on Feb 28, 2024

    [fc] Repository: plone.restapi

    d478eb3

    [fc] Repository: plone.restapi

    1180c8c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    3.12only security fixesonly security fixes3.13only security fixesonly security fixesstdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions

      You are visitor 01402503 — GeezerVison does not store the pages you look up.
      Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.