Skip to content

Refactor admin action test suite#35806

Merged
Joerger merged 3 commits intomasterfrom
joerger/admin-action-test-suite
Dec 19, 2023
Merged

Refactor admin action test suite#35806
Joerger merged 3 commits intomasterfrom
joerger/admin-action-test-suite

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Dec 16, 2023

Refactor admin action tctl tests to use testify/suite for shared setup/teardown.

This should unblock my open PRs (ex: #35464) where the flaky test detector is timing out trying to diff all test cases.

@github-actions github-actions Bot requested review from gzdunek and tigrato December 16, 2023 00:42
@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.

@github-actions github-actions Bot added size/sm tctl tctl - Teleport admin tool labels Dec 16, 2023
@Joerger Joerger added the no-changelog Indicates that a PR does not require a changelog entry label Dec 16, 2023
@Joerger Joerger requested a review from rosstimothy December 16, 2023 00:44
@Joerger Joerger force-pushed the joerger/admin-action-test-suite branch 2 times, most recently from 8e1ab94 to 2cd717e Compare December 16, 2023 00:56
@Joerger Joerger force-pushed the joerger/admin-action-test-suite branch from 2cd717e to 9b7909c Compare December 16, 2023 00:59
Copy link
Copy Markdown
Collaborator

@zmb3 zmb3 left a comment

Choose a reason for hiding this comment

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

Why exactly do we need this? It looks like we now have 3 or 4 levels of nested subtests which seems a bit much.

@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Dec 18, 2023

@zmb3 Without this refactor, the flaky test detector is timing out on my PRs trying to run all test subcases of TestAdminActionMFA. The flaky test detector has a condition for testify/suites where it only runs the changed subcases instead of all of them.

As for the deeper sub cases, I could remove a couple layers if we think it's too confusing and not worth the shared code.

@zmb3
Copy link
Copy Markdown
Collaborator

zmb3 commented Dec 18, 2023

The flaky test detector has a condition for testify/suites where it only runs the changed subcases instead of all of them.

Got it - that's the piece I was missing. Thanks for clarifying.

@Joerger Joerger enabled auto-merge December 18, 2023 20:34
@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Dec 19, 2023

@r0mant @zmb3 Can I get a flaky test skip?

@r0mant
Copy link
Copy Markdown
Collaborator

r0mant commented Dec 19, 2023

/excludeflake *

@Joerger Joerger added this pull request to the merge queue Dec 19, 2023
Merged via the queue into master with commit 09e828c Dec 19, 2023
@Joerger Joerger deleted the joerger/admin-action-test-suite branch December 19, 2023 01:25
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 tctl tctl - Teleport admin tool

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants