ci: fix CI workflow chain, Makefile build errors, and add test group workflows#5590
ci: fix CI workflow chain, Makefile build errors, and add test group workflows#5590renecannao merged 1 commit intov3.0from
Conversation
Workflow trigger chain fix: - All 24 test workflows now trigger on CI-builds completion instead of CI-trigger completion, ensuring build artifacts are available before tests start New test group workflows: - Add CI-legacy-g1, CI-legacy-g3, CI-legacy-g5 - Add CI-mysql84-g1 through CI-mysql84-g5 - All use the generic ci-taptests-groups.yml reusable workflow groups.json linter: - Add lint_groups_json.py enforcing compact one-line-per-entry format - Add CI-lint-groups-json.yml workflow triggered on groups.json changes - Reformat groups.json to comply with documented format Makefile fixes: - Fix $(error) call with unescaped ')' that broke all docker-compose builds since Release 3.0.8 - Fix CURVER extraction regex to handle 'v' prefix from git describe CI-taptests-pgsql-cluster rewrite: - Replace old jenkins-build-scripts with Unified CI infrastructure - Use ensure-infras.bash + run-tests-isolated.bash Infrastructure improvements: - Add --hostname test-runner to test-runner container for cross-container DNS resolution - Pass TAP_PGSQL_SYNC_REPLICA_PORT through to test container - Default TAP_PGSQL_SYNC_REPLICA_PORT=6042 when cluster nodes active
📝 WalkthroughWalkthroughThis pull request updates GitHub Actions CI workflow triggers to reference Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Python linter for groups.json to enforce formatting and sorting standards, alongside updates to the test infrastructure for PostgreSQL cluster sync support. The Makefile was also adjusted to handle version strings with an optional 'v' prefix. Feedback suggests enhancing the new linter to explicitly check for and report duplicate keys, which are currently ignored by the sorting check.
| if keys_in_order != sorted(keys_in_order): | ||
| # Find first out-of-order key | ||
| for j in range(len(keys_in_order) - 1): | ||
| if keys_in_order[j] > keys_in_order[j + 1]: | ||
| errors.append( | ||
| f"Keys not sorted: '{keys_in_order[j + 1]}' " | ||
| f"should come before '{keys_in_order[j]}'" | ||
| ) | ||
| break |
There was a problem hiding this comment.
The linter currently verifies that keys are sorted, but it does not check for duplicate keys. In JSON, duplicate keys are generally discouraged and most parsers (including Python's json.loads) will silently overwrite previous values with the last one encountered. Adding a check for uniqueness ensures that groups.json remains consistent and prevents accidental configuration overrides.
| if keys_in_order != sorted(keys_in_order): | |
| # Find first out-of-order key | |
| for j in range(len(keys_in_order) - 1): | |
| if keys_in_order[j] > keys_in_order[j + 1]: | |
| errors.append( | |
| f"Keys not sorted: '{keys_in_order[j + 1]}' " | |
| f"should come before '{keys_in_order[j]}'" | |
| ) | |
| break | |
| # Check 4: keys are sorted and unique | |
| if keys_in_order != sorted(keys_in_order): | |
| # Find first out-of-order key | |
| for j in range(len(keys_in_order) - 1): | |
| if keys_in_order[j] > keys_in_order[j + 1]: | |
| errors.append( | |
| f"Keys not sorted: '{keys_in_order[j + 1]}' " | |
| f"should come before '{keys_in_order[j]}'" | |
| ) | |
| break | |
| if len(keys_in_order) != len(set(keys_in_order)): | |
| seen = set() | |
| dupes = [k for k in keys_in_order if k in seen or seen.add(k)] | |
| errors.append(f"Duplicate keys found: {', '.join(sorted(set(dupes)))}") |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
test/infra/control/env-isolated.bash (1)
111-114: Use[[instead of[for the conditional test.The
[[construct is safer in bash as it prevents word splitting and pathname expansion issues. This aligns with the SonarCloud hint.🔧 Suggested fix
# Cluster sync test support — expose first cluster node admin port for replica validation -if [ "${NUM_CLUSTER_NODES}" -gt 0 ]; then +if [[ "${NUM_CLUSTER_NODES}" -gt 0 ]]; then export TAP_PGSQL_SYNC_REPLICA_PORT="${TAP_PGSQL_SYNC_REPLICA_PORT:-6042}" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/infra/control/env-isolated.bash` around lines 111 - 114, The conditional using [ "${NUM_CLUSTER_NODES}" -gt 0 ] should be changed to use the safer bash [[ ]] form; update the test that checks NUM_CLUSTER_NODES in the env-isolated.bash snippet (the block setting TAP_PGSQL_SYNC_REPLICA_PORT) to use [[ "${NUM_CLUSTER_NODES}" -gt 0 ]] so it avoids word-splitting/pathname expansion issues while preserving the same export behavior for TAP_PGSQL_SYNC_REPLICA_PORT.test/tap/groups/lint_groups_json.py (1)
23-115: Consider extracting validation checks into helper functions.The
main()function has high cognitive complexity (42 vs 15 recommended by SonarCloud). While acceptable for a linter script, extracting each check into separate functions would improve readability and testability:
validate_json_structure()validate_line_format()validate_key_order()validate_trailing_commas()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/tap/groups/lint_groups_json.py` around lines 23 - 115, The main() is doing multiple distinct validation steps; extract each into its own helper that returns a list of error strings and keep main() as the orchestrator: create validate_json_structure(raw) to parse JSON and verify top-level dict (use json.loads and return parsed data plus errors), validate_line_format(lines, entry_pattern) to verify first/last lines and per-line entry format (uses entry_pattern and populates keys_in_order), validate_key_order(keys_in_order) to check sorted order and return ordering errors, and validate_trailing_commas(lines, entry_pattern) to check commas on non-last/last entries; update main() to read file, compile entry_pattern once, call these helpers (collecting errors from each), and preserve existing final reporting/exit behavior (keep names main, entry_pattern, keys_in_order, errors to make locating code straightforward).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/CI-taptests-asan.yml:
- Around line 7-8: CI-codeql.yml still uses the old workflow trigger name
("workflows: [ CI-trigger ]") causing it to run out-of-order; update the
workflows trigger in CI-codeql.yml to match the new upstream workflow name
("workflows: [ CI-builds ]") so it runs after CI-builds completes and aligns
with the other downstream workflows, ensuring the "workflows:" key in
CI-codeql.yml references CI-builds rather than CI-trigger.
In `@test/tap/groups/lint_groups_json.py`:
- Around line 60-62: The two error messages appended in the lint_groups_json
check use f-strings without placeholders; update the calls that append to errors
(the lines using errors.append(f"Last non-empty line: must be '}}'") and
errors.append(f"Last line: must be '}}'")) to use plain string literals instead
(remove the leading f) so they are simple regular strings; keep the same text
and quoting and leave the surrounding logic that references lines and errors
unchanged.
---
Nitpick comments:
In `@test/infra/control/env-isolated.bash`:
- Around line 111-114: The conditional using [ "${NUM_CLUSTER_NODES}" -gt 0 ]
should be changed to use the safer bash [[ ]] form; update the test that checks
NUM_CLUSTER_NODES in the env-isolated.bash snippet (the block setting
TAP_PGSQL_SYNC_REPLICA_PORT) to use [[ "${NUM_CLUSTER_NODES}" -gt 0 ]] so it
avoids word-splitting/pathname expansion issues while preserving the same export
behavior for TAP_PGSQL_SYNC_REPLICA_PORT.
In `@test/tap/groups/lint_groups_json.py`:
- Around line 23-115: The main() is doing multiple distinct validation steps;
extract each into its own helper that returns a list of error strings and keep
main() as the orchestrator: create validate_json_structure(raw) to parse JSON
and verify top-level dict (use json.loads and return parsed data plus errors),
validate_line_format(lines, entry_pattern) to verify first/last lines and
per-line entry format (uses entry_pattern and populates keys_in_order),
validate_key_order(keys_in_order) to check sorted order and return ordering
errors, and validate_trailing_commas(lines, entry_pattern) to check commas on
non-last/last entries; update main() to read file, compile entry_pattern once,
call these helpers (collecting errors from each), and preserve existing final
reporting/exit behavior (keep names main, entry_pattern, keys_in_order, errors
to make locating code straightforward).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f7891421-8333-42ea-b7bf-c6a5b6188718
📒 Files selected for processing (38)
.github/workflows/CI-3p-aiomysql.yml.github/workflows/CI-3p-django-framework.yml.github/workflows/CI-3p-laravel-framework.yml.github/workflows/CI-3p-mariadb-connector-c.yml.github/workflows/CI-3p-mysql-connector-j.yml.github/workflows/CI-3p-pgjdbc.yml.github/workflows/CI-3p-php-pdo-mysql.yml.github/workflows/CI-3p-php-pdo-pgsql.yml.github/workflows/CI-3p-postgresql.yml.github/workflows/CI-3p-sqlalchemy.yml.github/workflows/CI-basictests.yml.github/workflows/CI-legacy-clickhouse-g1.yml.github/workflows/CI-legacy-g1.yml.github/workflows/CI-legacy-g2-genai.yml.github/workflows/CI-legacy-g2.yml.github/workflows/CI-legacy-g3.yml.github/workflows/CI-legacy-g4.yml.github/workflows/CI-legacy-g5.yml.github/workflows/CI-lint-groups-json.yml.github/workflows/CI-maketest.yml.github/workflows/CI-mysql84-g1.yml.github/workflows/CI-mysql84-g2.yml.github/workflows/CI-mysql84-g3.yml.github/workflows/CI-mysql84-g4.yml.github/workflows/CI-mysql84-g5.yml.github/workflows/CI-repltests.yml.github/workflows/CI-selftests.yml.github/workflows/CI-shuntest.yml.github/workflows/CI-taptests-asan.yml.github/workflows/CI-taptests-groups.yml.github/workflows/CI-taptests-pgsql-cluster.yml.github/workflows/CI-taptests-ssl.yml.github/workflows/CI-taptests.ymlMakefiletest/infra/control/env-isolated.bashtest/infra/control/run-tests-isolated.bashtest/tap/groups/groups.jsontest/tap/groups/lint_groups_json.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: claude-review
- GitHub Check: lint
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-04-01T21:27:00.297Z
Learnt from: wazir-ahmed
Repo: sysown/proxysql PR: 5557
File: test/tap/tests/unit/gtid_set_unit-t.cpp:14-17
Timestamp: 2026-04-01T21:27:00.297Z
Learning: In ProxySQL's unit test directory (test/tap/tests/unit/), test_globals.h and test_init.h are only required for tests that depend on the ProxySQL runtime globals/initialization (i.e., tests that exercise components linked against libproxysql.a). Pure data-structure or utility tests (e.g., ezoption_parser_unit-t.cpp, gtid_set_unit-t.cpp, gtid_trxid_interval_unit-t.cpp) only need tap.h and the relevant project header — omitting test_globals.h and test_init.h is correct and intentional in these cases.
Applied to files:
test/infra/control/run-tests-isolated.bashtest/infra/control/env-isolated.bashtest/tap/groups/groups.json
📚 Learning: 2026-03-26T16:39:02.446Z
Learnt from: yuji-hatakeyama
Repo: sysown/proxysql PR: 5548
File: lib/mysql_connection.cpp:1837-1843
Timestamp: 2026-03-26T16:39:02.446Z
Learning: In ProxySQL's lib/mysql_connection.cpp, `SHOW WARNINGS` detection for both `update_warning_count_from_connection()` and the `add_eof()` call in `ASYNC_USE_RESULT_CONT` intentionally uses `myds->sess->CurrentQuery.QueryParserArgs.digest_text` (comment-stripped digest text). This means the fix/feature does not work when `mysql-query_digests_keep_comment=1` (digest_text contains comments) or `mysql-query_digests=0` (digest_text is unavailable) — these configurations are explicitly excluded from the regression test for `reg_test_5306-show_warnings_with_comment-t`. This design is consistent across the codebase and is an accepted, documented limitation.
Applied to files:
test/tap/groups/groups.json
🪛 GitHub Check: SonarCloud Code Analysis
test/infra/control/env-isolated.bash
[failure] 112-112: Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich.
test/tap/groups/lint_groups_json.py
[warning] 66-66: Replace spaces with quantifier {2}.
[warning] 60-60: Add replacement fields or use a normal string instead of an f-string.
[warning] 62-62: Add replacement fields or use a normal string instead of an f-string.
[failure] 23-23: Refactor this function to reduce its Cognitive Complexity from 42 to the 15 allowed.
🪛 Ruff (0.15.9)
test/tap/groups/lint_groups_json.py
[error] 60-60: f-string without any placeholders
Remove extraneous f prefix
(F541)
[error] 62-62: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (40)
Makefile (2)
75-75: LGTM! Critical fix for version extraction.The addition of
v?to the regex correctly handles the optional leading 'v' prefix fromgit describeoutput. This resolves the CURVER extraction failures mentioned in the PR objectives, as GIT_VERSION may contain a leading 'v' when no feature flags are set or in recursive make invocations.
81-81: LGTM! Critical syntax fix resolves Makefile build failure.This change fixes the unescaped parenthesis issue mentioned in the PR objectives. The previous error message likely contained an unescaped
)that prematurely closed the$(error)function call, causing Makefile syntax errors. The revised message properly terminates the error call and remains clear and helpful..github/workflows/CI-legacy-g2-genai.yml (1)
7-7: Trigger remap toCI-buildslooks correct.This aligns the workflow with the build-completion gate and preserves existing completion/success safeguards.
.github/workflows/CI-3p-aiomysql.yml (1)
12-12:workflow_runupstream switch is valid.Using
CI-buildshere is consistent with the intended CI chain and keeps downstream execution conditions intact..github/workflows/CI-legacy-clickhouse-g1.yml (1)
7-7: Good trigger update toCI-builds.This preserves current behavior while ensuring this workflow waits for build completion.
.github/workflows/CI-selftests.yml (1)
7-7: LGTM on upstream workflow target change.Pointing
workflow_runtoCI-buildsis consistent and avoids pre-build race conditions..github/workflows/CI-3p-php-pdo-pgsql.yml (1)
12-12: Approved:workflow_runnow waits onCI-builds.The change is correct and keeps existing job conditions and inputs untouched.
.github/workflows/CI-3p-pgjdbc.yml (1)
12-12: Trigger update is correct and safe.Switching to
CI-buildsis compatible with the existingifexpression and branch exclusion logic..github/workflows/CI-3p-mysql-connector-j.yml (1)
12-12: Looks good: upstream trigger now points toCI-builds.This matches the workflow-chain intent and keeps current job behavior unchanged.
.github/workflows/CI-legacy-g2.yml (1)
7-7: Approved trigger remap for legacy group workflow.Using
CI-buildshere is consistent and correctly enforces build-first ordering..github/workflows/CI-3p-postgresql.yml (1)
12-12: Workflow dependency update looks correct.Switching the
workflow_runsource toCI-buildsis consistent with the new CI chain and should prevent early test starts..github/workflows/CI-3p-sqlalchemy.yml (1)
12-12: Trigger remap is good.This correctly points the workflow to
CI-buildscompletion, matching the intended sequencing..github/workflows/CI-taptests.yml (1)
7-7: Good workflow chaining change.Using
CI-buildsas the upstream trigger is the right dependency for this test workflow..github/workflows/CI-basictests.yml (1)
7-7: Dependency name is correctly updated.This now listens to the actual build workflow (
CI-builds), which is consistent with the CI orchestration update..github/workflows/CI-legacy-g4.yml (1)
7-7: Looks good.The trigger now depends on
CI-buildscompletion, which matches the new workflow chain behavior..github/workflows/CI-shuntest.yml (1)
7-7: Approved trigger update.This correctly re-anchors the workflow to
CI-buildscompletion..github/workflows/CI-taptests-ssl.yml (1)
7-7: CI trigger change is solid.Switching to
CI-buildsshould preserve ordering and avoid race conditions with build availability..github/workflows/CI-taptests-groups.yml (1)
7-7: This remap is correct.
workflow_runnow tracksCI-builds, which is the right upstream for grouped taptests execution..github/workflows/CI-3p-laravel-framework.yml (1)
12-13: LGTM!The trigger update from
CI-triggertoCI-buildsis correct and aligns with the PR objective to ensure test workflows wait for build artifacts to be available before running..github/workflows/CI-repltests.yml (1)
7-8: LGTM!Trigger update is consistent with the other workflow changes in this PR.
.github/workflows/CI-lint-groups-json.yml (1)
11-17: LGTM!The workflow is well-structured with appropriate path filtering. The use of
ubuntu-latestdefault Python should be sufficient for a simple JSON linting script..github/workflows/CI-maketest.yml (1)
7-8: LGTM!Trigger update is consistent with the workflow chain fix.
.github/workflows/CI-3p-php-pdo-mysql.yml (1)
12-13: LGTM!Trigger update aligns with the other CI-3p workflow changes.
.github/workflows/CI-3p-django-framework.yml (1)
12-13: LGTM!Trigger update is consistent with the PR's workflow chain fix.
.github/workflows/CI-3p-mariadb-connector-c.yml (1)
12-12: Trigger remap toCI-buildsis correct and aligns with the CI chain fix.
This keeps downstream execution gated on completed build artifacts as intended.test/infra/control/run-tests-isolated.bash (2)
237-237: Setting a stable runner hostname is a good infra hardening tweak.
This makes container identity deterministic for cross-container name resolution in isolated runs.
260-260: PassingTAP_PGSQL_SYNC_REPLICA_PORTinto the container is the right plumbing change.
This cleanly enables the pgsql cluster sync test path while preserving unset behavior..github/workflows/CI-mysql84-g3.yml (1)
1-21: Workflow wiring looks good formysql84-g3.
Triggering onCI-buildscompletion with the success guard and reusable workflow invocation is consistent with the intended CI chain..github/workflows/CI-mysql84-g2.yml (1)
1-21:mysql84-g2workflow configuration is consistent and correct.
The trigger, concurrency, and reusable workflow parameters match the new grouped CI approach..github/workflows/CI-legacy-g5.yml (1)
1-21:legacy-g5workflow setup looks solid.
It correctly gates execution on successfulCI-buildscompletion and uses the shared group runner workflow..github/workflows/CI-legacy-g1.yml (1)
1-21:legacy-g1workflow is correctly wired.
The event triggers, success condition, and testgroup payload are consistent with the rest of the CI grouping changes..github/workflows/CI-taptests-pgsql-cluster.yml (1)
1-93: The new pgsql-cluster workflow is well-structured and resilient.
The sequence (cache restore → infra start → isolated test run → unconditional cleanup → failure-only log archive) is robust and aligns with the unified CI migration goals..github/workflows/CI-mysql84-g4.yml (1)
1-21:mysql84-g4workflow definition looks correct.
It cleanly integrates into theCI-builds-driven grouped test workflow chain..github/workflows/CI-mysql84-g5.yml (2)
21-21: Test group "mysql84-g5" has zero tests mapped in groups.json.The workflow passes
testgroup: '["mysql84-g5"]', butgroups.jsoncontains no entries mapped to this group. According to the context provided:
mysql84-g1: 78 entriesmysql84-g2: 45 entriesmysql84-g3: 45 entriesmysql84-g4: 71 entriesmysql84-g5: 0 entriesThis workflow will complete successfully but run zero tests. If this is intentional (placeholder for future tests), consider adding a comment. Otherwise, tests need to be assigned to this group.
[raise_major_issue, request_verification]
#!/bin/bash # Verify mysql84-g5 has no test entries in groups.json echo "Counting entries for each mysql84-g* group:" for g in g1 g2 g3 g4 g5; do count=$(rg -c "\"mysql84-$g\"" test/tap/groups/groups.json 2>/dev/null || echo "0") echo " mysql84-$g: $count" done
1-20: Workflow structure follows established pattern.The workflow configuration (triggers, concurrency, and conditional execution) is consistent with CI-mysql84-g1 through CI-mysql84-g4, which is good for maintainability.
.github/workflows/CI-mysql84-g1.yml (1)
1-21: LGTM!The workflow is well-structured with proper triggers, concurrency handling, and conditional execution. The
mysql84-g1test group is properly populated with 78 test entries ingroups.json.test/tap/groups/lint_groups_json.py (1)
1-21: LGTM!The script provides comprehensive validation of
groups.jsonformat including JSON validity, line format, key sorting, and trailing comma rules. The structure is clear and the error messages are helpful..github/workflows/CI-legacy-g3.yml (1)
1-21: LGTM!The workflow follows the established pattern for test group workflows. The
legacy-g3group has tests mapped ingroups.json(including the newtest_cluster_sync_pgsql-tentry).test/tap/groups/groups.json (2)
273-273: Verifytest_cluster_sync_pgsql-ttest binary exists.This entry references
test_cluster_sync_pgsql-t, but the context indicates that onlytest_cluster_sync-t.cppexists in the codebase (which is MySQL-specific, usingmysql.hand MySQL-related APIs). There appears to be no correspondingtest_cluster_sync_pgsql-t.cppsource file.If the test binary doesn't exist, CI workflows running
legacy-g3will fail or silently skip this test.[raise_major_issue, request_verification]
#!/bin/bash # Check if test_cluster_sync_pgsql source or binary references exist echo "Searching for test_cluster_sync_pgsql source files:" fd -i "test_cluster_sync_pgsql" --type f echo "" echo "Searching for references in Makefiles or CMake:" rg -l "test_cluster_sync_pgsql" --type cmake --type make 2>/dev/null || rg -l "test_cluster_sync_pgsql" -g "Makefile*" -g "CMake*" echo "" echo "Searching for any mention in the codebase:" rg -l "test_cluster_sync_pgsql" --type-not json
221-237: Entry reordering for alphabetical compliance.These entries were reordered to satisfy the alphabetical sorting requirement enforced by the new
lint_groups_json.pylinter. The group mappings remain unchanged.
| workflows: [ CI-builds ] | ||
| types: [ completed ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find all workflows still referencing CI-trigger to identify any orphaned references
echo "=== Workflows referencing CI-trigger ==="
rg -l 'workflows:.*CI-trigger' .github/workflows/ || echo "None found"
echo ""
echo "=== Workflows referencing CI-builds ==="
rg -l 'workflows:.*CI-builds' .github/workflows/ || echo "None found"Repository: sysown/proxysql
Length of output: 1439
Trigger update is correct, but CI-codeql.yml was not updated.
This workflow correctly triggers on CI-builds completion. However, .github/workflows/CI-codeql.yml still references workflows: [ CI-trigger ] and remains on the old trigger path.
This creates an inconsistency in the workflow chain: CI-codeql.yml will run in parallel with (not after) CI-builds completes, while 29+ other downstream workflows now wait for CI-builds to succeed. If the intent is for all validation to complete before dependent workflows run, CI-codeql.yml should also be updated to trigger on CI-builds completion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/CI-taptests-asan.yml around lines 7 - 8, CI-codeql.yml
still uses the old workflow trigger name ("workflows: [ CI-trigger ]") causing
it to run out-of-order; update the workflows trigger in CI-codeql.yml to match
the new upstream workflow name ("workflows: [ CI-builds ]") so it runs after
CI-builds completes and aligns with the other downstream workflows, ensuring the
"workflows:" key in CI-codeql.yml references CI-builds rather than CI-trigger.
| errors.append(f"Last non-empty line: must be '}}'") | ||
| elif lines[-1].strip() != "}": | ||
| errors.append(f"Last line: must be '}}'") |
There was a problem hiding this comment.
Remove extraneous f prefixes from strings without placeholders.
Static analysis correctly identifies that these f-strings have no placeholders. Use regular strings instead.
🐛 Proposed fix
- errors.append(f"Last non-empty line: must be '}}'")
+ errors.append("Last non-empty line: must be '}'")
elif lines[-1].strip() != "}":
- errors.append(f"Last line: must be '}}'")
+ errors.append("Last line: must be '}'")🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 60-60: Add replacement fields or use a normal string instead of an f-string.
[warning] 62-62: Add replacement fields or use a normal string instead of an f-string.
🪛 Ruff (0.15.9)
[error] 60-60: f-string without any placeholders
Remove extraneous f prefix
(F541)
[error] 62-62: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/tap/groups/lint_groups_json.py` around lines 60 - 62, The two error
messages appended in the lint_groups_json check use f-strings without
placeholders; update the calls that append to errors (the lines using
errors.append(f"Last non-empty line: must be '}}'") and errors.append(f"Last
line: must be '}}'")) to use plain string literals instead (remove the leading
f) so they are simple regular strings; keep the same text and quoting and leave
the surrounding logic that references lines and errors unchanged.
1 participant
Summary
$(error)had unescaped)breaking all docker-compose builds, and CURVER regex didn't handlevprefix fromgit describe--hostname test-runnerfor cross-container DNS, passTAP_PGSQL_SYNC_REPLICA_PORTto test containerTest plan
Summary by CodeRabbit
Release Notes
New Features
Chores