googleapis / googleapis/google-api-python-client

test: connection leak test test_discovery_http_is_closed is shadowed and syntactically invalid

Đang mở
#2,757 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
Python
Star
8.9k
Fork
2.6k
Merge trung bình
2 ngày 28 phút
Pull request đã merge (30 ngày)
17

Mô tả

### **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:
```python
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.

```python
@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'
```

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.