Skip to content

Commit 2fe971e

Browse files
authored
Fix race condition in integration test cleanup (#513)
## Problem I noticed that tests were failing for the python 3.13 build of `tests/integration/data_grpc_futures` tests after merging PR #510 to main. ## Solution Context: - When tests are run on a PR branch, they only execute on python 3.9. - When tests they run on main, tests run for both python 3.9 and 3.13. What I observed: - In this case, the python 3.13 build of newly added tests `tests/integration/data_grpc_futures` was consistently failing with "Unauthorized" errors - Despite the confusing error message from the API, this "Unauthorized" response was being returned when making calls for an index that no longer exists. - Why did the index no longer exist? Because the python 3.9 build of the same tests was completing first and nuking the index before the 3.13 python build completes. - Why would the 3.9 tests delete an index being used by the 3.13 tests? - A test helper was assigning a deterministic name for the index that was the same for both builds. Solution: - Revert back to a randomized name strategy for indexes. Need to rethink deterministic names later before we can use something like VCR to playback recorded API calls in testing. - Reenable 3.13 tests on PR branches for now so we can see that this change takes care of the problem observed on merge ## Type of Change - [x] Bug fix (non-breaking change which fixes an issue) - [x] Infrastructure change (CI configs, etc) ## Test Plan Should see all tests passing even when 3.9 and 3.13 tests are run together on PR push.
1 parent 86c381c commit 2fe971e

File tree

3 files changed

+12
-4
lines changed

3 files changed

+12
-4
lines changed

.github/workflows/on-pr.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ jobs:
4848
- create-project
4949
with:
5050
encrypted_project_api_key: ${{ needs.create-project.outputs.encrypted_project_api_key }}
51-
python_versions_json: '["3.9"]'
51+
python_versions_json: '["3.13", "3.9"]'
5252

5353
cleanup-project:
5454
if: ${{ always() }}

tests/integration/helpers/helpers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ def poll_stats_for_namespace(
7575
idx: _Index,
7676
namespace: str,
7777
expected_count: int,
78-
max_sleep: int = int(os.environ.get("FRESHNESS_TIMEOUT_SECONDS", 60)),
78+
max_sleep: int = int(os.environ.get("FRESHNESS_TIMEOUT_SECONDS", 180)),
7979
) -> None:
8080
delta_t = 5
8181
total_time = 0

tests/integration/helpers/names.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import hashlib
1+
import uuid
2+
# import hashlib
23

34

45
def generate_name(test_name: str, label: str, max_length: int = 20) -> str:
@@ -11,4 +12,11 @@ def generate_name(test_name: str, label: str, max_length: int = 20) -> str:
1112
since the full length of 64 characters exceeds the allowed length of some fields
1213
in the API. For example, index names must be 45 characters or less.
1314
"""
14-
return hashlib.sha256(f"{test_name}-{label}".encode()).hexdigest()[:max_length]
15+
# return hashlib.sha256(f"{test_name}-{label}".encode()).hexdigest()[:max_length]
16+
17+
# Having names be fully deterministic led to problems when multiple test builds
18+
# are running in parallel, for example running the same tests in parallel for
19+
# different python versions. We can solve this by incorporating more information
20+
# into the name generation, but for now as a quick fix to unblock a release we
21+
# will just fall back to using a random uuid.
22+
return str(uuid.uuid4())

0 commit comments

Comments
 (0)