Skip to content

test: connection leak test test_discovery_http_is_closed is shadowed and syntactically invalid #2757

@Capstan

Description

@Capstan

Environment details

  • OS type and version: Linux (gLinux / Debian based)
  • Python version: 3.13.12 (also affects all supported Python versions 3.10+)
  • pip version: 26.0.1
  • google-api-python-client version: 2.197.0 (at commit 6e471c075039dfef24e28d11658e03d5c949c7c3)

Steps to reproduce

  1. Look at tests/test_discovery.py on main. Note that there are two separate definitions for class Discovery(unittest.TestCase) (one at line 498 and one at line 1566).
  2. In Python, defining a class twice in the same module causes the second definition to silently overwrite the first. This means the first Discovery class—which contains the test case test_discovery_http_is_closed—is completely dead code and is never executed by pytest.
  3. Attempting to manually extract and execute this test case crashes immediately because HttpMock.close() is a regular function, but the test tries to call assert_called_once() on it.

Code example

The shadowed class in tests/test_discovery.py contains this test definition:

class Discovery(unittest.TestCase):
    def test_discovery_http_is_closed(self):
        http = HttpMock(datafile("malformed.json"), {"status": "200"})
        # build() is called without passing the 'http' mock client!
        service = build("plus", "v1", credentials=mock.sentinel.credentials)
        # Fails: HttpMock.close is a standard method, not a mock object
        http.close.assert_called_once()

Historical Context

This bug was introduced in September 2020 by commit 98888dadf ("fix: add method to close httplib2 connections (#1038)").
That commit added test_discovery_http_is_closed by declaring a new class Discovery(unittest.TestCase) block near the top of the file, without realizing that a class Discovery(unittest.TestCase) block was already defined and active near the bottom of the file (which immediately shadowed the new test).

The Proposed Solution

  1. Delete the duplicate class definition at line 498.
  2. Restore test_discovery_http_is_closed inside the active Discovery class (at line 1566).
  3. Correct the test case to use standard unittest.mock.patch on httplib2.Http to verify that the temporary HTTP client created by build() to fetch the discovery document is safely closed after initialization completes, preventing connection/socket leaks.
    @mock.patch("httplib2.Http")
    def test_discovery_http_is_closed(self, mock_http_class):
        mock_http = mock_http_class.return_value
        mock_http.request.return_value = (
            httplib2.Response({"status": "200"}),
            read_datafile("plus.json"),
        )
        service = build("plus", "v1", static_discovery=False)
        mock_http.close.assert_called_once()

Stack trace

If you isolate the legacy test case and run it, it crashes immediately with an AttributeError:

AttributeError: 'function' object has no attribute 'assert_called_once'

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions