Skip to content

Conversation

@jhamon
Copy link
Collaborator

@jhamon jhamon commented Jun 13, 2025

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

  • Bug fix (non-breaking change which fixes an issue)
  • 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.

@jhamon jhamon force-pushed the jhamon/test-investigation branch from ec97553 to b4a6f88 Compare June 16, 2025 14:59
@jhamon jhamon marked this pull request as ready for review June 16, 2025 15:11
@jhamon jhamon changed the title Test investigation Fix race condition in test cleanup Jun 16, 2025
@jhamon jhamon requested a review from rohanshah18 June 16, 2025 15:12
@jhamon jhamon changed the title Fix race condition in test cleanup Fix race condition in integration test cleanup Jun 16, 2025
Copy link
Contributor

@rohanshah18 rohanshah18 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@rohanshah18 rohanshah18 merged commit 2fe971e into main Jun 16, 2025
35 checks passed
@rohanshah18 rohanshah18 deleted the jhamon/test-investigation branch June 16, 2025 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants