Skip to content

test: use gotestsum to retry only the failing test #1990

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.

Sign up for GitHub

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

Closed
alepane21 wants to merge 6 commits into from
Closed

test: use gotestsum to retry only the failing test #1990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a798520
test: use gotestsum to retry only the failing test and not the whole …
alepane21 Jun 25, 2025
e189544
chore: fix variables
alepane21 Jun 25, 2025
c93bfce
chore: rerun only flaky tests
alepane21 Jun 25, 2025
559772a
chore: reintroduce matrix usage to split flaky and non-flaky tests
alepane21 Jun 25, 2025
8f0950d
chore: remove p and parallel limitation from not-flaky tests
alepane21 Jun 25, 2025
e934524
chore: make integrations test matrix more readable
alepane21 Jun 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 5 additions & 18 deletions .github/workflows/router-ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,9 @@ jobs:
strategy:
fail-fast: false
matrix:
test_target:
[
'./. ./fuzzquery ./lifecycle ./modules',
'./telemetry',
'./events',
]
make_target:
- integration-tests
- integration-tests-flaky
services:
Comment on lines +142 to 145
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 10, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Trailing spaces break YAML-lint & block merges

YAML-lint is already flagging these lines:

Remove the blanks to keep the pipeline green.

-        make_target: 
-          - integration-tests
-          - integration-tests-flaky
+        make_target:
+          - integration-tests
+          - integration-tests-flaky
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
make_target:
- integration-tests
- integration-tests-flaky
services:
make_target:
- integration-tests
- integration-tests-flaky
services:
🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 142-142: trailing spaces

(trailing-spaces)

🤖 Prompt for AI Agents 
In .github/workflows/router-ci.yaml around lines 142 to 145, there are trailing
spaces after the list items under make_target which cause YAML-lint errors and
block merges. Remove any trailing spaces at the end of these lines to ensure the
YAML syntax is clean and the pipeline passes linting checks.

nats:
image: ghcr.io/wundergraph/cosmo/nats:2.11.0-alpine
Expand Down Expand Up @@ -222,19 +219,9 @@ jobs:
redis-cli -u "redis://cosmo:test@127.0.0.1:$port" ping
echo "ACL user 'cosmo' created with full access on port $port"
done
- name: Run Integration tests ${{ matrix.test_target }}
- name: Run Integration Tests
working-directory: ./router-tests
run: make test test_params="-run '^Test[^(Flaky)]' --timeout=5m -p 1 --parallel 10" test_target="${{ matrix.test_target }}"
- name: Run Flaky Integration tests ${{ matrix.test_target }}
uses: nick-fields/retry@v3
with:
timeout_minutes: 30
max_attempts: 5
retry_wait_seconds: 5
retry_on: error
command: |
cd router-tests
make test test_params="-run '^TestFlaky' --timeout=5m --parallel 1" test_target="${{ matrix.test_target }}"
run: make ${{ matrix.make_target }}

image_scan:
if: github.event.pull_request.head.repo.full_name == github.repository
Expand Down
8 changes: 7 additions & 1 deletion router-tests/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ test-deps:
$(MAKE) -C ../demo plugin-build-ci

test: test-deps
gotestsum -f $(FORMAT) -- -ldflags=-extldflags=-Wl,-ld_classic $(test_params) -race $(test_target)
gotestsum $(gotestsum_params) --packages=$(test_target) -f $(FORMAT) -- -ldflags=-extldflags=-Wl,-ld_classic $(test_params) -race $(test_target)

Comment on lines +14 to 15
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 10, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid passing the package list twice & quote optional params

gotestsum already receives the package list through --packages=$(test_target).
The trailing $(test_target) positional arg is redundant and can break when the
variable is empty/over-ridden. In addition, any of the parameter variables
containing spaces should be quoted to prevent word-splitting.

-	gotestsum $(gotestsum_params) --packages=$(test_target) -f $(FORMAT) -- -ldflags=-extldflags=-Wl,-ld_classic $(test_params) -race $(test_target)
+	gotestsum "$(gotestsum_params)" --packages="$(test_target)" -f "$(FORMAT)" -- -ldflags=-extldflags=-Wl,-ld_classic $(test_params) -race
🤖 Prompt for AI Agents 
In router-tests/Makefile around lines 14 to 15, remove the redundant positional
package argument at the end of the gotestsum command since the packages are
already specified with --packages=$(test_target). Also, quote the parameter
variables like "$(gotestsum_params)" and "$(test_params)" to prevent
word-splitting issues when they contain spaces.

update-snapshot:
go test -update -race $(test_target)
Expand Down Expand Up @@ -44,3 +44,9 @@ update-test-config:

bump-deps:
./bump-deps.sh

integration-tests:
$(MAKE) test test_params="-run '^Test[^(Flaky)]' --timeout=10m" test_target="./..."

Comment on lines +48 to +50
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 10, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Regex does not exclude “Flaky” tests – it matches only one extra rune

^Test[^(Flaky)] is a character class, i.e. it allows exactly one
character that is not “(”, “F”, “l”, “a”, “k”, or “y”.
It will not exclude TestFlaky*; instead it runs almost nothing.

Go test (RE2) lacks negative look-ahead, so a common workaround is:

-	$(MAKE) test test_params="-run '^Test[^(Flaky)]' --timeout=10m" test_target="./..."
+	# run all tests whose name does **not** start with TestFlaky
+	$(MAKE) test test_params="-run '^Test([^F]|F[^l]|Fl[^a]|Fla[^k]|Flak[^y])' --timeout=10m" test_target="./..."

Alternatively, move flaky tests into a separate package or use build tags for
clean separation.
Failing to fix this will leave the non-flaky suite virtually empty.

🤖 Prompt for AI Agents 
In router-tests/Makefile around lines 48 to 50, the regex `^Test[^(Flaky)]`
incorrectly uses a character class and does not exclude tests with "Flaky" in
their names, causing almost no tests to run. Replace this regex with a pattern
that matches test names starting with "Test" but excludes those containing
"Flaky" by using a workaround such as `^Test[^F].*` or another suitable pattern
that avoids matching "Flaky" tests, or alternatively separate flaky tests into a
different package or use build tags for exclusion.

integration-tests-flaky:
$(MAKE) test gotestsum_params="--rerun-fails=5" test_params="-run '^TestFlaky' --timeout=10m -p 1 --parallel 10" test_target="./..."
Loading