[TT-12494] Fix flaky TestCacheEtag and related cache tests

titpetric commented 2 months ago

User description

This considers some necessary improvements:

[x] revert test to run e2e-combined (e2e only has some CI specific issues when running, not seen on local)
[x] add cancellation group to CI tests, new commits on PR cancel old CI Tests run
[x] pass only the merged .cov file to sonarcloud (not all of them)
[x] https://tyktech.atlassian.net/browse/TT-12494 (tests issue, deleting caches, defer)
[x] fix some dialing out in tests/regression (httpbin/google)

gateway_test.go: The tests are flaky on account they delete the redis cache. Deleting a redis cache from a test interferes with other tests that also read/write cache data from redis.

tests/regression: using external resources (confirmed fix)

PR Type

enhancement, tests

Description

Added concurrency settings to the CI workflow to ensure only one runner per PR and commit, canceling old runs on new commits.
Updated the test command to test:e2e-combined to address CI-specific issues.
Changed the sonar coverage report path to use a merged .cov file (gateway-all.cov).
Simplified the package listing command in the test task configuration.

Changes walkthrough 📝

Relevant files

Enhancement

ci-tests.yml `Enhance CI workflow with concurrency and test improvements` .github/workflows/ci-tests.yml Added concurrency settings to CI tests to cancel old runs on new commits. Changed test command to `test:e2e-combined`. Updated sonar coverage report path to `gateway-all.cov`.	+11/-2
test.yml `Simplify package listing command in test task` .taskfiles/test.yml - Simplified the package listing command in the test task.	+1/-1

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

github-actions[bot] commented 2 months ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ No key issues to review

github-actions[bot] commented 2 months ago

PR Code Suggestions ✨

Category	Suggestion	Score
Possible issue	Check and ensure the `task` command is installed before using it ___ Ensure that the `task` command is installed and available in the CI environment before calling `task --version`, to prevent potential failures during the execution of the workflow. [.github/workflows/ci-tests.yml [97]](https://github.com/TykTechnologies/tyk/pull/6508/files#diff-03609cb60b0c6e92fb771eb8787d6722b8c31ca4c03eabc788e147acd8c6fb43R97-R97) ```diff -task --version +- name: Ensure task is installed + run: \| + if ! command -v task &> /dev/null + then + echo "task could not be found, installing..." + # Add installation command here + fi +- name: Check task version + run: task --version ``` Suggestion importance[1-10]: 8 Why: Ensuring the `task` command is installed before using it is crucial to prevent workflow failures, making this a significant improvement for reliability.	8
Enhancement	Use a configurable environment variable for the test timeout setting ___ Replace the hardcoded timeout value with a configurable environment variable or a workflow input to make the timeout setting more flexible and maintainable. [.github/workflows/ci-tests.yml [129]](https://github.com/TykTechnologies/tyk/pull/6508/files#diff-03609cb60b0c6e92fb771eb8787d6722b8c31ca4c03eabc788e147acd8c6fb43R129-R129) ```diff -task test:e2e-combined args="-race -timeout=15m" +task test:e2e-combined args="-race -timeout=${{ env.TEST_TIMEOUT }}" ``` Suggestion importance[1-10]: 7 Why: Replacing the hardcoded timeout with a configurable variable enhances flexibility and maintainability, though it is not critical for functionality.	7
Best practice	Use a more precise pattern for coverage report paths to ensure accuracy ___ Specify a more precise coverage report path pattern to ensure that only relevant coverage files are included, avoiding potential inclusion of unwanted coverage data. [.github/workflows/ci-tests.yml [156]](https://github.com/TykTechnologies/tyk/pull/6508/files#diff-03609cb60b0c6e92fb771eb8787d6722b8c31ca4c03eabc788e147acd8c6fb43R156-R156) ```diff --Dsonar.go.coverage.reportPaths=coverage/gateway-all.cov +-Dsonar.go.coverage.reportPaths=coverage/gateway-*.cov ``` Suggestion importance[1-10]: 6 Why: Specifying a more precise pattern for coverage report paths can improve accuracy, but the impact is relatively minor unless there are multiple coverage files that could be incorrectly included.	6
Best practice	Narrow the pattern matching for release branches to prevent unintended CI triggers ___ Consider using a more specific pattern for the release branches in the CI triggers to avoid unintentional builds from non-release branches that might use similar naming conventions. [.github/workflows/ci-tests.yml [18]](https://github.com/TykTechnologies/tyk/pull/6508/files#diff-03609cb60b0c6e92fb771eb8787d6722b8c31ca4c03eabc788e147acd8c6fb43R18-R18) ```diff branches: - master - - release-** + - release-* ``` Suggestion importance[1-10]: 5 Why: The suggestion to narrow the pattern for release branches can help avoid unintended CI triggers, but the improvement is minor since it may not significantly impact the workflow unless there are similarly named branches.	5