Skip to content

fix: Gitea test TestGiteaParamsChangedFilesCEL failure#2046

Closed
zakisk wants to merge 0 commit intotektoncd:mainfrom
zakisk:fix-gitea-test-flakiness
Closed

fix: Gitea test TestGiteaParamsChangedFilesCEL failure#2046
zakisk wants to merge 0 commit intotektoncd:mainfrom
zakisk:fix-gitea-test-flakiness

Conversation

@zakisk
Copy link
Copy Markdown
Member

@zakisk zakisk commented Apr 15, 2025

This test was failing after #2039 PR is merged.

Issue

TestGiteaParamsChangedFilesCEL test merges a PR in order to trigger PipelineRun on push of PR merge, so before the #2039 PR the PipelineRun was cancelling (it was a bug) but after its fix no PipelineRun was cancelling and an event is being emitted by cancelAllInProgressBelongingToClosedPullRequest func and the failing test TestGiteaParamsChangedFilesCEL was checking that no event is emitted but there was one event.

Solution

Simply skip checking for events as it doesn't have any significance in the test.

Changes

Submitter Checklist

  • 📝 Ensure your commit message is clear and informative. Refer to the How to write a git commit message guide. Include the commit message in the PR body rather than linking to an external site (e.g., Jira ticket).

  • ♽ Run make test lint before submitting a PR to avoid unnecessary CI processing. Consider installing pre-commit and running pre-commit install in the repository root for an efficient workflow.

  • ✨ We use linters to maintain clean and consistent code. Run make lint before submitting a PR. Some linters offer a --fix mode, executable with make fix-linters (ensure markdownlint and golangci-lint are installed).

  • 📖 Document any user-facing features or changes in behavior.

  • 🧪 While 100% coverage isn't required, we encourage unit tests for code changes where possible.

  • 🎁 If feasible, add an end-to-end test. See README for details.

  • 🔎 Address any CI test flakiness before merging, or provide a valid reason to bypass it (e.g., token rate limitations).

  • If adding a provider feature, fill in the following details:

    • GitHub App
    • GitHub Webhook
    • Gitea/Forgejo
    • GitLab
    • Bitbucket Cloud
    • Bitbucket Data Center

    (update the provider documentation accordingly)

@zakisk zakisk force-pushed the fix-gitea-test-flakiness branch from e44d78d to 96befb8 Compare April 15, 2025 09:33
Comment thread test/pkg/gitea/test.go Outdated
assert.Assert(t, len(events.Items) != 0, "events expected in case of failure but got 0")
} else if !topts.SkipEventsCheck {
assert.Assert(t, len(events.Items) == 0, fmt.Sprintf("no events expected but got %v in %v ns, items: %+v", len(events.Items), topts.TargetNS, events.Items))
assert.Assert(t, len(events.Items) == 1, fmt.Sprintf("no events expected but got %v in %v ns, items: %+v", len(events.Items), topts.TargetNS, events.Items))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i don't think that's right... there is something happening with regard to cancellation and we can't just assume == 1 here...

@zakisk zakisk closed this Apr 16, 2025
@zakisk zakisk force-pushed the fix-gitea-test-flakiness branch from 96befb8 to c2b17f0 Compare April 16, 2025 11:10
@zakisk zakisk deleted the fix-gitea-test-flakiness branch April 25, 2025 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants