test: fix retry helpers currently causing flaky test failures (#1369) · googleapis/python-spanner@c55fb36 · GitHub | Latest TMZ Celebrity News & Gossip | Watch TMZ Live
Skip to content

Commit c55fb36

Browse files
authored
test: fix retry helpers currently causing flaky test failures (#1369)
Fix the retry helpers that are currently causing multiple tests to fail randomly.
1 parent 3943885 commit c55fb36

File tree

5 files changed

+52
-22
lines changed

5 files changed

+52
-22
lines changed

noxfile.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@
5151
"pytest-cov",
5252
"pytest-asyncio",
5353
]
54+
MOCK_SERVER_ADDITIONAL_DEPENDENCIES = [
55+
"google-cloud-testutils",
56+
]
5457
UNIT_TEST_EXTERNAL_DEPENDENCIES: List[str] = []
5558
UNIT_TEST_LOCAL_DEPENDENCIES: List[str] = []
5659
UNIT_TEST_DEPENDENCIES: List[str] = []
@@ -242,8 +245,11 @@ def mockserver(session):
242245
constraints_path = str(
243246
CURRENT_DIRECTORY / "testing" / f"constraints-{session.python}.txt"
244247
)
245-
# install_unittest_dependencies(session, "-c", constraints_path)
246-
standard_deps = UNIT_TEST_STANDARD_DEPENDENCIES + UNIT_TEST_DEPENDENCIES
248+
standard_deps = (
249+
UNIT_TEST_STANDARD_DEPENDENCIES
250+
+ UNIT_TEST_DEPENDENCIES
251+
+ MOCK_SERVER_ADDITIONAL_DEPENDENCIES
252+
)
247253
session.install(*standard_deps, "-c", constraints_path)
248254
session.install("-e", ".", "-c", constraints_path)
249255

tests/mockserver_tests/test_aborted_transaction.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
14+
import random
1415

1516
from google.cloud.spanner_v1 import (
1617
BatchCreateSessionsRequest,
@@ -29,6 +30,12 @@
2930
add_update_count,
3031
add_single_result,
3132
)
33+
from google.api_core import exceptions
34+
from test_utils import retry
35+
36+
retry_maybe_aborted_txn = retry.RetryErrors(
37+
exceptions.Aborted, max_tries=5, delay=0, backoff=1
38+
)
3239

3340

3441
class TestAbortedTransaction(MockServerTestBase):
@@ -119,6 +126,18 @@ def test_batch_commit_aborted(self):
119126
# The transaction is aborted and retried.
120127
self.assertTrue(isinstance(requests[2], CommitRequest))
121128

129+
@retry_maybe_aborted_txn
130+
def test_retry_helper(self):
131+
# Randomly add an Aborted error for the Commit method on the mock server.
132+
if random.random() < 0.5:
133+
add_error(SpannerServicer.Commit.__name__, aborted_status())
134+
session = self.database.session()
135+
session.create()
136+
transaction = session.transaction()
137+
transaction.begin()
138+
transaction.insert("my_table", ["col1, col2"], [{"col1": 1, "col2": "One"}])
139+
transaction.commit()
140+
122141

123142
def _insert_mutations(transaction: Transaction):
124143
transaction.insert("my_table", ["col1", "col2"], ["value1", "value2"])

tests/system/_helpers.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@
7474
retry_429_503 = retry.RetryErrors(
7575
exceptions.TooManyRequests, exceptions.ServiceUnavailable, 8
7676
)
77-
retry_mabye_aborted_txn = retry.RetryErrors(exceptions.ServerError, exceptions.Aborted)
78-
retry_mabye_conflict = retry.RetryErrors(exceptions.ServerError, exceptions.Conflict)
77+
retry_maybe_aborted_txn = retry.RetryErrors(exceptions.Aborted)
78+
retry_maybe_conflict = retry.RetryErrors(exceptions.Conflict)
7979

8080

8181
def _has_all_ddl(database):

tests/system/test_dbapi.py

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -763,12 +763,15 @@ def test_commit_abort_retry(self, dbapi_database):
763763
dbapi_database._method_abort_interceptor.set_method_to_abort(
764764
COMMIT_METHOD, self._conn
765765
)
766-
# called 2 times
766+
# called (at least) 2 times
767767
self._conn.commit()
768768
dbapi_database._method_abort_interceptor.reset()
769-
assert method_count_interceptor._counts[COMMIT_METHOD] == 2
770-
assert method_count_interceptor._counts[EXECUTE_BATCH_DML_METHOD] == 4
771-
assert method_count_interceptor._counts[EXECUTE_STREAMING_SQL_METHOD] == 10
769+
# Verify the number of calls.
770+
# We don't know the exact number of calls, as Spanner could also
771+
# abort the transaction.
772+
assert method_count_interceptor._counts[COMMIT_METHOD] >= 2
773+
assert method_count_interceptor._counts[EXECUTE_BATCH_DML_METHOD] >= 4
774+
assert method_count_interceptor._counts[EXECUTE_STREAMING_SQL_METHOD] >= 10
772775

773776
self._cursor.execute("SELECT * FROM contacts")
774777
got_rows = self._cursor.fetchall()
@@ -829,10 +832,12 @@ def test_execute_sql_abort_retry_multiple_times(self, dbapi_database):
829832
self._cursor.fetchmany(2)
830833
dbapi_database._method_abort_interceptor.reset()
831834
self._conn.commit()
832-
# Check that all rpcs except commit should be called 3 times the original
833-
assert method_count_interceptor._counts[COMMIT_METHOD] == 1
834-
assert method_count_interceptor._counts[EXECUTE_BATCH_DML_METHOD] == 3
835-
assert method_count_interceptor._counts[EXECUTE_STREAMING_SQL_METHOD] == 3
835+
# Check that all RPCs except commit should be called at least 3 times
836+
# We don't know the exact number of attempts, as the transaction could
837+
# also be aborted by Spanner (and not only the test interceptor).
838+
assert method_count_interceptor._counts[COMMIT_METHOD] >= 1
839+
assert method_count_interceptor._counts[EXECUTE_BATCH_DML_METHOD] >= 3
840+
assert method_count_interceptor._counts[EXECUTE_STREAMING_SQL_METHOD] >= 3
836841

837842
self._cursor.execute("SELECT * FROM contacts")
838843
got_rows = self._cursor.fetchall()

tests/system/test_session_api.py

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ def test_batch_insert_w_commit_timestamp(sessions_database, not_postgres):
578578
assert not deleted
579579

580580

581-
@_helpers.retry_mabye_aborted_txn
581+
@_helpers.retry_maybe_aborted_txn
582582
def test_transaction_read_and_insert_then_rollback(
583583
sessions_database,
584584
ot_exporter,
@@ -687,7 +687,7 @@ def test_transaction_read_and_insert_then_rollback(
687687
)
688688

689689

690-
@_helpers.retry_mabye_conflict
690+
@_helpers.retry_maybe_conflict
691691
def test_transaction_read_and_insert_then_exception(sessions_database):
692692
class CustomException(Exception):
693693
pass
@@ -714,7 +714,7 @@ def _transaction_read_then_raise(transaction):
714714
assert rows == []
715715

716716

717-
@_helpers.retry_mabye_conflict
717+
@_helpers.retry_maybe_conflict
718718
def test_transaction_read_and_insert_or_update_then_commit(
719719
sessions_database,
720720
sessions_to_delete,
@@ -771,8 +771,8 @@ def _generate_insert_returning_statement(row, database_dialect):
771771
return f"INSERT INTO {table} ({column_list}) VALUES ({row_data}) {returning}"
772772

773773

774-
@_helpers.retry_mabye_conflict
775-
@_helpers.retry_mabye_aborted_txn
774+
@_helpers.retry_maybe_conflict
775+
@_helpers.retry_maybe_aborted_txn
776776
def test_transaction_execute_sql_w_dml_read_rollback(
777777
sessions_database,
778778
sessions_to_delete,
@@ -809,7 +809,7 @@ def test_transaction_execute_sql_w_dml_read_rollback(
809809
# [END spanner_test_dml_rollback_txn_not_committed]
810810

811811

812-
@_helpers.retry_mabye_conflict
812+
@_helpers.retry_maybe_conflict
813813
def test_transaction_execute_update_read_commit(sessions_database, sessions_to_delete):
814814
# [START spanner_test_dml_read_your_writes]
815815
sd = _sample_data
@@ -838,7 +838,7 @@ def test_transaction_execute_update_read_commit(sessions_database, sessions_to_d
838838
# [END spanner_test_dml_read_your_writes]
839839

840840

841-
@_helpers.retry_mabye_conflict
841+
@_helpers.retry_maybe_conflict
842842
def test_transaction_execute_update_then_insert_commit(
843843
sessions_database, sessions_to_delete
844844
):
@@ -870,7 +870,7 @@ def test_transaction_execute_update_then_insert_commit(
870870
# [END spanner_test_dml_with_mutation]
871871

872872

873-
@_helpers.retry_mabye_conflict
873+
@_helpers.retry_maybe_conflict
874874
@pytest.mark.skipif(
875875
_helpers.USE_EMULATOR, reason="Emulator does not support DML Returning."
876876
)
@@ -901,7 +901,7 @@ def test_transaction_execute_sql_dml_returning(
901901
sd._check_rows_data(rows)
902902

903903

904-
@_helpers.retry_mabye_conflict
904+
@_helpers.retry_maybe_conflict
905905
@pytest.mark.skipif(
906906
_helpers.USE_EMULATOR, reason="Emulator does not support DML Returning."
907907
)
@@ -929,7 +929,7 @@ def test_transaction_execute_update_dml_returning(
929929
sd._check_rows_data(rows)
930930

931931

932-
@_helpers.retry_mabye_conflict
932+
@_helpers.retry_maybe_conflict
933933
@pytest.mark.skipif(
934934
_helpers.USE_EMULATOR, reason="Emulator does not support DML Returning."
935935
)

0 commit comments

Comments
 (0)

TMZ Celebrity News – Breaking Stories, Videos & Gossip

Looking for the latest TMZ celebrity news? You've come to the right place. From shocking Hollywood scandals to exclusive videos, TMZ delivers it all in real time.

Whether it’s a red carpet slip-up, a viral paparazzi moment, or a legal drama involving your favorite stars, TMZ news is always first to break the story. Stay in the loop with daily updates, insider tips, and jaw-dropping photos.

🎥 Watch TMZ Live

TMZ Live brings you daily celebrity news and interviews straight from the TMZ newsroom. Don’t miss a beat—watch now and see what’s trending in Hollywood.