Skip to content

Deflake HTTP_PROXY tests#33614

Merged
atburke merged 1 commit intomasterfrom
atburke/http-proxy-tests-flake
Nov 8, 2023
Merged

Deflake HTTP_PROXY tests#33614
atburke merged 1 commit intomasterfrom
atburke/http-proxy-tests-flake

Conversation

@atburke
Copy link
Copy Markdown
Contributor

@atburke atburke commented Oct 18, 2023

This change rewrites a few HTTP_PROXY related tests to be less flaky.

Resolves #33197.

@github-actions github-actions Bot added size/sm tsh tsh - Teleport's command line tool for logging into nodes running Teleport. labels Oct 18, 2023
@github-actions github-actions Bot requested review from bl-nero and tigrato October 18, 2023 00:07
Comment thread api/testhelpers/proxy.go Outdated
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we avoid variable name hiding here, and say, use tt? I was surprised seeing the proxyHandler.Reset passed to the cleanup function and was wondering how the hell we expect to get back to between loop iterations.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Are you referring to t? Shadowing t in subtests is quite common and is actually the preferred approach.

If you have tt and t in scope, it's possible to mistakenly use the wrong one and (for example) fail the top-level test instead of the subtest. Shadowing t makes that impossible because the outer t is no longer accessible to the closure.

It's even written this way in the official docs for the testing package: https://pkg.go.dev/testing#hdr-Subtests_and_Sub_benchmarks

Comment thread api/client/webclient/webclient_test.go
@github-actions
Copy link
Copy Markdown
Contributor

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@atburke atburke added the no-changelog Indicates that a PR does not require a changelog entry label Oct 23, 2023
@atburke atburke force-pushed the atburke/http-proxy-tests-flake branch from d3db604 to 5b0b8ea Compare October 31, 2023 23:36
@atburke
Copy link
Copy Markdown
Contributor Author

atburke commented Oct 31, 2023

Friendly ping @bl-nero @zmb3

@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from tigrato November 2, 2023 09:23
@atburke atburke force-pushed the atburke/http-proxy-tests-flake branch from 65cd42e to 9aea0cb Compare November 7, 2023 21:28
@atburke atburke enabled auto-merge November 7, 2023 21:29
@atburke atburke added this pull request to the merge queue Nov 7, 2023
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 7, 2023
This change rewrites a few HTTP_PROXY tests to be less flaky.
@atburke atburke force-pushed the atburke/http-proxy-tests-flake branch from 9aea0cb to dd3e3f4 Compare November 8, 2023 00:23
@atburke atburke enabled auto-merge November 8, 2023 00:23
@atburke atburke added this pull request to the merge queue Nov 8, 2023
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 8, 2023
@atburke atburke added this pull request to the merge queue Nov 8, 2023
Merged via the queue into master with commit 42822da Nov 8, 2023
@atburke atburke deleted the atburke/http-proxy-tests-flake branch November 8, 2023 17:26
@public-teleport-github-review-bot
Copy link
Copy Markdown

@atburke See the table below for backport results.

Branch Result
branch/v12 Failed
branch/v13 Failed
branch/v14 Create PR

atburke added a commit that referenced this pull request Nov 8, 2023
This change rewrites a few HTTP_PROXY tests to be less flaky.
atburke added a commit that referenced this pull request Nov 8, 2023
This change rewrites a few HTTP_PROXY tests to be less flaky.
github-merge-queue Bot pushed a commit that referenced this pull request Nov 17, 2023
This change rewrites a few HTTP_PROXY tests to be less flaky.
github-merge-queue Bot pushed a commit that referenced this pull request Nov 17, 2023
This change rewrites a few HTTP_PROXY tests to be less flaky.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a PR does not require a changelog entry size/sm tsh tsh - Teleport's command line tool for logging into nodes running Teleport.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TestNewWebClientNoProxy flakiness

3 participants