Skip to content

Commit 15f7bca

Browse files
authored
Unit test fix: restore monkey-patched datetime when test finishes (#3001)
## Changes This PR updates a unit test that monkey-patched the system `datetime` implementation but did not restore the original when the test finished. Depending on the order of test collection, execution, and allocation across pytest workers this can affect subsequent tests. ### Linked issues Broke (unit) tests in #2743. ### Tests - updated unit tests
1 parent 8f6400f commit 15f7bca

File tree

1 file changed

+27
-4
lines changed

1 file changed

+27
-4
lines changed

tests/unit/hive_metastore/test_table_migrate.py

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import datetime
22
import logging
33
import sys
4+
from collections.abc import Generator
45
from itertools import cycle
56
from unittest.mock import create_autospec
67

@@ -909,14 +910,36 @@ def test_is_upgraded(ws, mock_pyspark):
909910
external_locations.resolve_mount.assert_not_called()
910911

911912

912-
def test_table_status():
913-
class FakeDate(datetime.datetime):
913+
@pytest.fixture
914+
def datetime_with_epoch_timestamp(monkeypatch) -> Generator[None, None, None]:
915+
# The timestamp() method on datetime() is immutable, so we can't just patch/mock it. Rather we need to substitute
916+
# the entire class, which the normal mocking/patching routines don't seem to support.
917+
# Note: this will not affect any modules that have already initialized and imported the class directly. Similarly,
918+
# the effect cannot be unwound if this test triggers initializing of modules that import the class directly instead
919+
# of the module.
920+
# If you find yourself here debugging an issue, please be aware that the following can be problematic:
921+
# >>> from datetime import datetime
922+
# (Why? It's reference to the class directly, and having the name of the module makes inspection tedious.)
923+
# Prefer instead:
924+
# >>> import datetime
925+
# Or even better:
926+
# >>> import datetime as dt
927+
928+
class FakeDateTime(datetime.datetime):
914929

915930
def timestamp(self):
916931
return 0
917932

918-
datetime.datetime = FakeDate
919-
errors = {}
933+
_original_datetime = datetime.datetime
934+
try:
935+
datetime.datetime = FakeDateTime # type: ignore[misc]
936+
yield
937+
finally:
938+
datetime.datetime = _original_datetime # type: ignore[misc]
939+
940+
941+
def test_table_status(datetime_with_epoch_timestamp) -> None:
942+
errors: dict[str, str] = {}
920943
rows = {
921944
"SHOW TBLPROPERTIES `schema1`.`table1`": MockBackend.rows("key", "value")["upgrade_to", "cat1.schema1.dest1"]
922945
}

0 commit comments

Comments
 (0)