Enhancement smoke - #497
Conversation
📝 WalkthroughWalkthroughThe PR updates smoke test tooling: GNUmakefile's smoke-tests target forwards SMOKE_OPTS into the runner. smoke/_init.sh now deletes any existing grout namespace before creating it and adds a PAUSE_ON_FAILURE-driven interactive pause during cleanup that can print SMOKE_LOG and show a debug command. smoke/run.sh adds -m/--match and -p/--pause-on-failure options, introduces match_patterns and pause_on_failure state, changes match/skip logic for selecting tests, passes PAUSE_ON_FAILURE and SMOKE_LOG into tests, and suppresses log output when pausing on failure. No actionable comments were generated in the recent review. 🎉 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@smoke/run.sh`:
- Around line 57-58: The for-loop using "for script in $here/$match_pattern; do"
can iterate once with the literal pattern when there are no matches; add a
pre-check before that loop to detect zero matches and exit with a clear error.
For example, expand the glob into positional parameters (or enable nullglob) and
test whether the first entry equals the unexpanded pattern or whether the match
list is empty; if empty, print a concise error and exit non‑zero (referencing
the variables "here" and "match_pattern" and the loop that iterates over
"script" and uses "basename"). Ensure the subsequent loop only runs when there
is at least one real match.
| for script in $here/$match_pattern; do | ||
| name=$(basename $script) |
There was a problem hiding this comment.
Fail fast when no scripts match the -m pattern.
If the glob matches nothing, the loop runs once with the literal pattern and produces a misleading “No such file” failure. It’s better to detect zero matches and exit with a clear message.
Proposed fix
-for script in $here/$match_pattern; do
+scripts=($here/$match_pattern)
+if [ ${`#scripts`[@]} -eq 0 ]; then
+ echo "error: no tests match pattern '$match_pattern'" >&2
+ exit 1
+fi
+for script in ${scripts[@]}; do📝 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.
| for script in $here/$match_pattern; do | |
| name=$(basename $script) | |
| scripts=($here/$match_pattern) | |
| if [ ${`#scripts`[@]} -eq 0 ]; then | |
| echo "error: no tests match pattern '$match_pattern'" >&2 | |
| exit 1 | |
| fi | |
| for script in ${scripts[@]}; do | |
| name=$(basename $script) |
🤖 Prompt for AI Agents
In `@smoke/run.sh` around lines 57 - 58, The for-loop using "for script in
$here/$match_pattern; do" can iterate once with the literal pattern when there
are no matches; add a pre-check before that loop to detect zero matches and exit
with a clear error. For example, expand the glob into positional parameters (or
enable nullglob) and test whether the first entry equals the unexpanded pattern
or whether the match list is empty; if empty, print a concise error and exit
non‑zero (referencing the variables "here" and "match_pattern" and the loop that
iterates over "script" and uses "basename"). Ensure the subsequent loop only
runs when there is at least one real match.
| -m|--match) | ||
| shift | ||
| match_pattern="$1" | ||
| ;; |
There was a problem hiding this comment.
Could you change this to work like -s|--skip ? (e.g. allow multiple -m arguments). You can rework skip_test or add a match_test function to check the patterns.
6515aa8 to
1702896
Compare
Delete the grout network namespace before creating it to ensure a clean state. This prevents leftover kernel devices from crashed previous runs causing errors when creating interfaces. Signed-off-by: Maxime Leroy <maxime@leroys.fr> Reviewed-by: Robin Jarry <rjarry@redhat.com>
Add -p|--pause-on-failure option to run.sh that pauses execution when a test fails and prints the grcli command with the socket path. This allows inspecting grout state interactively before the test cleanup runs. Signed-off-by: Maxime Leroy <maxime@leroys.fr> Reviewed-by: Robin Jarry <rjarry@redhat.com>
Add -m|--match option to run.sh that specifies a glob pattern to select which tests to run. Defaults to *_test.sh to run all tests. Signed-off-by: Maxime Leroy <maxime@leroys.fr> Reviewed-by: Robin Jarry <rjarry@redhat.com>
Allow passing options to run.sh via the SMOKE_OPTS variable, for example: make smoke-tests SMOKE_OPTS="-p -m '*vrf*'" Signed-off-by: Maxime Leroy <maxime@leroys.fr> Reviewed-by: Robin Jarry <rjarry@redhat.com>
1702896 to
5440657
Compare
Few enhancements to ease the life of developer using smoke tests to debug.
Summary by CodeRabbit
New Features
Chores