Loosen the gitlab_runner connection-failure exception assertion#24298
Draft
mwdd146980 wants to merge 1 commit into
Draft
Loosen the gitlab_runner connection-failure exception assertion#24298mwdd146980 wants to merge 1 commit into
mwdd146980 wants to merge 1 commit into
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
Contributor
Validation Report
Run Passed validations (20)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Loosens
test_connection_failureingitlab_runner/tests/test_integration.pyfrom assertingrequests.exceptions.ConnectionErrorto a barepytest.raises(Exception), and removes the now-unusedrequestsimport.Motivation
Part of the httpx2 migration.
test_connection_failuretriggers a genuine connection error against a real closed socket, so the exception type raised is an implementation detail of which HTTP backend (requeststoday,httpxafter the migration flip) is active. Pinning to the agnostic exception type isn't possible yet since nothing on this path normalizes exceptions before the httpx backend flip, and pinning torequests.exceptions.ConnectionErrorwould go red once the backend flips for no production reason. The two service checks the failure actually produces (gitlab_runner.can_connectandgitlab_runner.prometheus_endpoint_up, both asserted CRITICAL) are the observable behavior and are unchanged.No production code is touched; only the test file changes.
Verification
ddev test -fs gitlab_runner:ruff formatandruff checkboth pass, no lint warning from the removed import.grep -rn requests gitlab_runner/tests/test_integration.pyreturns no hits.ddev --no-interactive test gitlab_runner -- -k test_connection_failurecould not complete locally: thedd_environmentfixture's GitLab CE container (gitlab-ce:10.8.0-ce.0, an amd64-only 2018 image) did not finish booting within the fixture's retry budget under emulation on this Apple Silicon host, across two ~10-minute attempts. This is an environment limitation, not a code issue — the failure occurs in fixture setup, before the test body (and the changed assertion) ever executes. CI runs on x86 Linux and should not hit this. Requesting CI confirmtest_connection_failurepasses.Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged