From e3d2cc37076a4bc81d9027a5937441b31a43a060 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Fri, 16 Jan 2026 22:09:58 +0000 Subject: [PATCH] Fix issues reported by AI code review agents Address feedback from gemini-code-assist and coderabbitai: 1. Fix hardcoded paths in all test scripts: - Use relative paths from script location - Allow PROXYSQL_PATH to be overridden via environment variable - Updated: test_strict_mode.sh, test_config_validation.sh, quick_test.sh 2. Fix hardcoded paths in documentation: - Replace /home/rene/proxysql_5263/ with / - Updated: TEST_SUMMARY.md 3. Fix deprecated variable test file: - Add header comment listing deprecated variables - Mark have_compress with inline DEPRECATED comment - Updated: deprecated_variables.ini 4. Fix TAP test plan counts: - Change to plan(NO_PLAN) since tests vary based on ProxySQL mode - Updated: test_strict_config_validation-t.cpp, test_strict_pgsql_validation-t.cpp 5. Fix CI script logic bug in README.md: - Remove conflicting set -e - Use proper if statement to capture exit code - Updated: README.md Note: Reviews about implementation code (ProxySQL_Config.cpp) are not applicable as that code hasn't been implemented yet - the PR is still WIP. --- test/config_validation/README.md | 5 +---- test/config_validation/TEST_SUMMARY.md | 12 ++++++------ test/config_validation/deprecated_variables.ini | 5 ++++- test/config_validation/quick_test.sh | 13 ++++++++----- test/config_validation/test_config_validation.sh | 8 +++++--- test/config_validation/test_strict_mode.sh | 8 +++++--- test/tap/tests/test_strict_config_validation-t.cpp | 4 ++-- test/tap/tests/test_strict_pgsql_validation-t.cpp | 4 ++-- 8 files changed, 33 insertions(+), 26 deletions(-) diff --git a/test/config_validation/README.md b/test/config_validation/README.md index 9a35367a0..7002c5fdb 100644 --- a/test/config_validation/README.md +++ b/test/config_validation/README.md @@ -189,12 +189,9 @@ The test suite can be integrated into CI/CD pipelines: ```bash #!/bin/bash # Example CI test script -set -e # Run validation tests -./test_config_validation.sh - -if [ $? -eq 0 ]; then +if ./test_config_validation.sh; then echo "All configuration validation tests passed!" exit 0 else diff --git a/test/config_validation/TEST_SUMMARY.md b/test/config_validation/TEST_SUMMARY.md index 76034f994..208e16758 100644 --- a/test/config_validation/TEST_SUMMARY.md +++ b/test/config_validation/TEST_SUMMARY.md @@ -102,7 +102,7 @@ test/tap/tests/ **Running the tests:** ```bash -cd /home/rene/proxysql_5263/test/config_validation +cd /test/config_validation ./test_strict_mode.sh ``` @@ -130,7 +130,7 @@ cd /home/rene/proxysql_5263/test/config_validation **Building and running:** ```bash -cd /home/rene/proxysql_5263/test/tap/tests +cd /test/tap/tests make test_strict_config_validation-t ./test_strict_config_validation-t ``` @@ -152,7 +152,7 @@ make test_strict_config_validation-t **Building and running:** ```bash -cd /home/rene/proxysql_5263/test/tap/tests +cd /test/tap/tests make test_strict_pgsql_validation-t ./test_strict_pgsql_validation-t ``` @@ -243,7 +243,7 @@ mysql> LOAD MYSQL SERVERS FROM CONFIG; ```bash # Build the TAP test framework -cd /home/rene/proxysql_5263/test/tap +cd /test/tap make # Build specific tests @@ -256,11 +256,11 @@ make test_strict_pgsql_validation-t ```bash # Run shell script tests (startup + CLI flags) -cd /home/rene/proxysql_5263/test/config_validation +cd /test/config_validation ./test_strict_mode.sh # Run C++ integration tests (runtime validation) -cd /home/rene/proxysql_5263/test/tap/tests +cd /test/tap/tests ./test_strict_config_validation-t ./test_strict_pgsql_validation-t diff --git a/test/config_validation/deprecated_variables.ini b/test/config_validation/deprecated_variables.ini index 4a5a5b1a0..29492d749 100644 --- a/test/config_validation/deprecated_variables.ini +++ b/test/config_validation/deprecated_variables.ini @@ -1,5 +1,8 @@ # Configuration file with deprecated variables # This tests that deprecated variables are properly handled in strict mode +# +# DEPRECATED VARIABLES USED IN THIS FILE: +# - have_compress (replaced by 'compression' in mysql_variables) # Global settings admin_variables = @@ -14,7 +17,7 @@ mysql_variables = max_connections = 2048 default_query_delay = 0 default_query_timeout = 36000000 - have_compress = true + have_compress = true # DEPRECATED: Use 'compression' instead poll_timeout = 2000 interfaces = "0.0.0.0:6033" default_schema = information_schema diff --git a/test/config_validation/quick_test.sh b/test/config_validation/quick_test.sh index f9c615128..6034186d0 100755 --- a/test/config_validation/quick_test.sh +++ b/test/config_validation/quick_test.sh @@ -3,14 +3,17 @@ # Quick Test Script for Configuration Validation # This script quickly tests various configuration scenarios -PROXYSQL_PATH="/home/rene/proxysql_5263/src/proxysql" -TEST_DIR="/home/rene/proxysql_5263/test/config_validation" +# Configuration - use relative paths from script location +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +TEST_DIR="$SCRIPT_DIR" +# Allow PROXYSQL_PATH to be overridden by environment variable +PROXYSQL_PATH="${PROXYSQL_PATH:-$SCRIPT_DIR/../../src/proxysql}" echo "Quick Configuration Validation Tests" echo "====================================" -# Change to test directory -cd "$TEST_DIR" +# Change to test directory with error handling +cd "$TEST_DIR" || { echo "ERROR: Cannot change to test directory: $TEST_DIR"; exit 1; } echo -e "\n1. Testing valid MySQL servers configuration..." timeout 5s "$PROXYSQL_PATH" --validate-only "valid_mysql_servers.ini" @@ -40,4 +43,4 @@ echo -e "\n7. Testing invalid field in PostgreSQL users..." timeout 5s "$PROXYSQL_PATH" --validate-only "invalid_pgsql_users.ini" echo "Exit code: $?" -echo -e "\nAll quick tests completed!" \ No newline at end of file +echo -e "\nAll quick tests completed!" diff --git a/test/config_validation/test_config_validation.sh b/test/config_validation/test_config_validation.sh index 01fd6b179..99e3438c7 100755 --- a/test/config_validation/test_config_validation.sh +++ b/test/config_validation/test_config_validation.sh @@ -9,9 +9,11 @@ echo "========================================" echo "ProxySQL Configuration Validation Tests" echo "========================================" -# Configuration -PROXYSQL_PATH="/home/rene/proxysql_5263/src/proxysql" -TEST_DIR="/home/rene/proxysql_5263/test/config_validation" +# Configuration - use relative paths from script location +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +TEST_DIR="$SCRIPT_DIR" +# Allow PROXYSQL_PATH to be overridden by environment variable +PROXYSQL_PATH="${PROXYSQL_PATH:-$SCRIPT_DIR/../../src/proxysql}" LOG_DIR="/tmp/config_validation_tests" RESULTS_FILE="$LOG_DIR/test_results.txt" diff --git a/test/config_validation/test_strict_mode.sh b/test/config_validation/test_strict_mode.sh index 4c24a46f3..ca9f9e4ad 100755 --- a/test/config_validation/test_strict_mode.sh +++ b/test/config_validation/test_strict_mode.sh @@ -9,9 +9,11 @@ echo "========================================" echo "ProxySQL --strict Flag Validation Tests" echo "========================================" -# Configuration -PROXYSQL_PATH="/home/rene/proxysql_5263/src/proxysql" -TEST_DIR="/home/rene/proxysql_5263/test/config_validation" +# Configuration - use relative paths from script location +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +TEST_DIR="$SCRIPT_DIR" +# Allow PROXYSQL_PATH to be overridden by environment variable +PROXYSQL_PATH="${PROXYSQL_PATH:-$SCRIPT_DIR/../../src/proxysql}" LOG_DIR="/tmp/strict_mode_tests" RESULTS_FILE="$LOG_DIR/strict_test_results.txt" diff --git a/test/tap/tests/test_strict_config_validation-t.cpp b/test/tap/tests/test_strict_config_validation-t.cpp index aa46712a6..82bdc24cc 100644 --- a/test/tap/tests/test_strict_config_validation-t.cpp +++ b/test/tap/tests/test_strict_config_validation-t.cpp @@ -168,8 +168,8 @@ int main(int argc, char** argv) { return EXIT_FAILURE; } - // Plan: 10 tests total - plan(10); + // NO_PLAN - number of tests varies based on ProxySQL mode (strict vs non-strict) + plan(NO_PLAN); MYSQL* admin = mysql_init(NULL); if (!admin) { diff --git a/test/tap/tests/test_strict_pgsql_validation-t.cpp b/test/tap/tests/test_strict_pgsql_validation-t.cpp index cb63fbbb2..0a4f59e9b 100644 --- a/test/tap/tests/test_strict_pgsql_validation-t.cpp +++ b/test/tap/tests/test_strict_pgsql_validation-t.cpp @@ -179,8 +179,8 @@ int main(int argc, char** argv) { return EXIT_FAILURE; } - // Plan: 12 tests total - plan(12); + // NO_PLAN - number of tests varies based on ProxySQL mode (strict vs non-strict) + plan(NO_PLAN); MYSQL* admin = mysql_init(NULL); if (!admin) {