From 568da0add2a648f53fd96ccfcb4ecb5133146648 Mon Sep 17 00:00:00 2001 From: Chmouel Boudjnah Date: Wed, 18 Mar 2026 16:17:17 +0100 Subject: [PATCH] test: use /test TestGiteaReTestAll (and rename it) The TestGiteaRetestAll E2E test was flaky due to a race condition between the initial pull_request event and the /retest comment event. Here is what was happening: 1. The test creates a PR, which triggers a PipelineRun 2. The test then posts /retest to re-run all pipelines 3. Both events (pull_request and retest) are processed concurrently 4. The /retest handler calls filterSuccessfulTemplates to skip pipelines that already succeeded 5. Because the initial PipelineRun completes very fast (it just runs "echo HELLOMOTO"), the retest filter sometimes sees it as already successful and skips it 6. This means only 1 out of 2 expected statuses are created, causing a timeout The fix is to use /test instead of /retest. The /test command re-runs all pipelines unconditionally, without checking if they already succeeded. This eliminates the race condition entirely. The test is also renamed from TestGiteaRetestAll to TestGiteaTestAll to reflect the new behavior being tested, and a stale TODO comment is removed. Signed-off-by: Chmouel Boudjnah Co-Authored-By: Claude Opus 4.6 Signed-off-by: Chmouel Boudjnah --- test/gitea_gitops_commands_test.go | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/test/gitea_gitops_commands_test.go b/test/gitea_gitops_commands_test.go index b22212e4e..a7bfc1fdc 100644 --- a/test/gitea_gitops_commands_test.go +++ b/test/gitea_gitops_commands_test.go @@ -180,7 +180,14 @@ func TestGiteaTestPipelineRunExplicitlyWithTestComment(t *testing.T) { assert.NilError(t, err) } -func TestGiteaRetestAll(t *testing.T) { +// TestGiteaTestAll creates a PR with two pipeline definitions (one matching, +// one non-matching), then posts a /test comment to re-trigger all matching +// pipelines. It verifies that the repository ends up with exactly 2 statuses: +// one from the initial push and one from the /test comment. +// NOTE: We use /test rather than /retest because /retest skips pipelines that +// already succeeded (via filterSuccessfulTemplates), which causes a race when +// the initial PipelineRun finishes before the retest filter runs. +func TestGiteaTestAll(t *testing.T) { var err error ctx := context.Background() topts := &tgitea.TestOpts{ @@ -200,7 +207,7 @@ func TestGiteaRetestAll(t *testing.T) { } _, f := tgitea.TestPR(t, topts) defer f() - tgitea.PostCommentOnPullRequest(t, topts, "/retest") + tgitea.PostCommentOnPullRequest(t, topts, "/test") waitOpts := twait.Opts{ RepoName: topts.TargetNS, Namespace: topts.TargetNS, @@ -210,14 +217,18 @@ func TestGiteaRetestAll(t *testing.T) { repo, err := twait.UntilRepositoryUpdated(context.Background(), topts.ParamsRun.Clients, waitOpts) assert.NilError(t, err) - var rt bool + var hasPullRequest bool + var hasTestAll bool for _, status := range repo.Status { - // TODO(chmouel): Revert back to opscomments.RetestAllCommentEventType.String(), as pull_request now due of https://issues.redhat.com/browse/SRVKP-5775 if *status.EventType == triggertype.PullRequest.String() { - rt = true + hasPullRequest = true + } + if *status.EventType == opscomments.TestAllCommentEventType.String() { + hasTestAll = true } } - assert.Assert(t, rt, "should have a retest all comment event in status") + assert.Assert(t, hasPullRequest, "should have the initial pull request event in status") + assert.Assert(t, hasTestAll, "should have a test all comment event in status") assert.Equal(t, len(repo.Status), 2, "should have only 2 status") }