Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reenable the UDS proxy tests. #16189

Closed
wants to merge 1 commit into from
Closed

Reenable the UDS proxy tests. #16189

wants to merge 1 commit into from

Conversation

tjgq
Copy link
Contributor

@tjgq tjgq commented Aug 29, 2022

These tests were disabled due to flakiness in 8e3c0df. As a consequence, we
introduced an undetected regression in 5.3 (#16171).

I've rerun the tests multiple times, both locally and remotely, and I've seen
no evidence of flakiness. Therefore, I'm declaring that this issue has
mysteriously fixed itself.

Notes:

  • Use python3 instead of python to run the UDS proxy, as the latter is no
    longer available in some of the environments where the tests run.
  • The correct URI syntax for a Unix domain socket URI is unix:///path/to/socket
    rather than unix:/path/to/socket, as the URI technically has no host
    component. The latter form still works due to lenient parsing in the gRPC
    gRPC library, but this may change in the future, so the tests should exercise
    the correct form.

Fixes #16185.

@tjgq tjgq force-pushed the uds-tests branch 2 times, most recently from ebdeb54 to 8d63c1f Compare August 29, 2022 16:21
These tests were disabled due to flakiness in 8e3c0df. As a consequence, we
introduced an undetected regression in 5.3 (bazelbuild#16171).

I've rerun the tests multiple times, both locally and remotely, and I've seen
no evidence of flakiness. Therefore, I'm declaring that this issue has
mysteriously fixed itself.

Notes:

* Use `python3` instead of `python` to run the UDS proxy, as the latter is no
  longer available in some of the environments where the tests run.
* The correct URI syntax for a Unix domain socket URI is unix:///path/to/socket
  rather than unix:/path/to/socket, as the URI technically has no host
  component. The latter form still works due to lenient parsing in the gRPC
  gRPC library, but this may change in the future, so the tests should exercise
  the correct form.

Fixes bazelbuild#16185.
@tjgq tjgq marked this pull request as ready for review August 29, 2022 16:27
@tjgq tjgq requested a review from a team as a code owner August 29, 2022 16:27
@ShreeM01 ShreeM01 added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Aug 29, 2022
@coeuvre
Copy link
Member

coeuvre commented Jan 18, 2023

Closing as we discussed offline that the test is still flaky. Feel free to reopen if you have time to look at this again.

@coeuvre coeuvre closed this Jan 18, 2023
@tjgq tjgq deleted the uds-tests branch March 7, 2024 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review PR is awaiting review from an assigned reviewer team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reenable tests for gRPC UDS proxy
3 participants