Skip to content

Conversation

@rjarry
Copy link
Collaborator

@rjarry rjarry commented Oct 23, 2025

  • cli: force line buffering
  • main: print logs to stdout
  • smoke: colorize output
  • smoke: use /dev/urandom instead of SRANDOM

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Log output now correctly writes to standard output instead of error stream
  • Chores

    • Improved I/O buffering initialization with enhanced error handling
    • Enhanced test output with terminal-aware colorized logging and command tracing for better visibility

Set stdout and stderr to line buffered mode to ensure output is flushed
after each newline. This is needed when output is piped through awk for
colorization, as block buffering would delay output until the buffer
fills, making the smoke tests appear to hang.

Signed-off-by: Robin Jarry <rjarry@redhat.com>
Keep stderr for actual errors.

Signed-off-by: Robin Jarry <rjarry@redhat.com>
When stdout is a TTY, colorize different output streams to improve
readability during test execution:

- Bash xtrace (set -x) in cyan
- grout daemon logs in blue
- grout daemon errors in bold red
- grcli events in yellow
- FRR zebra logs in magenta

Each stream is piped through awk to apply ANSI color codes.

Signed-off-by: Robin Jarry <rjarry@redhat.com>
Replace the bash-specific SRANDOM variable with /dev/urandom for
generating random test run identifiers. Reading from /dev/urandom
provides better randomness than SRANDOM and shortens the line.

Add || : to prevent pipeline failures from terminating the test script
when head closes the pipe early after reading one line.

Signed-off-by: Robin Jarry <rjarry@redhat.com>
@coderabbitai
Copy link

coderabbitai bot commented Oct 23, 2025

📝 Walkthrough

Walkthrough

This pull request modifies I/O handling and output formatting across the codebase. Changes include explicit line-buffered I/O initialization for stdout and stderr in the CLI main function with error handling, redirection of log output from stderr to stdout in the DPDK logging module, and introduction of tty-aware colorized logging and tracing in the shell test initialization script. The shell script updates conditionally apply color formatting to various subsystem outputs when stdout is a terminal.

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "smoke: colorize output" directly corresponds to one of the primary changes described in the PR—the introduction of tty-aware, colorized logging in smoke/_init.sh. While the PR also includes supporting changes to cli/main.c and main/dpdk.c (CLI buffering and log output redirection), these appear to be enablers for the smoke output improvements. The title is specific, clear, and accurately reflects a main aspect of the changeset without being vague or misleading.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3530a16 and 054c1d7.

📒 Files selected for processing (3)
  • cli/main.c (1 hunks)
  • main/dpdk.c (1 hunks)
  • smoke/_init.sh (4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{c,h}

⚙️ CodeRabbit configuration file

**/*.{c,h}: - gr_vec_*() functions cannot fail. No need to check their return value.

  • gr_vec_free(x) always sets x = NULL. There is no risk of double free.
  • ec_node_*() functions consume their ec_node arguments. No leaks on error.
  • rte_node->ctx is an uint8_t array of size 16, not a pointer.
  • Don't suggest to replace assert() with graceful error checking.
  • We compile with -std=gnu2x. Unnamed parameters in function definitions are valid.

Files:

  • cli/main.c
  • main/dpdk.c
**/*.sh

⚙️ CodeRabbit configuration file

**/*.sh: - Don't bother about unquoted shell variables.

Files:

  • smoke/_init.sh
🪛 Shellcheck (0.11.0)
smoke/_init.sh

[warning] 104-104: run_id appears unused. Verify use (or export if used externally).

(SC2034)

⏰ 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). (8)
  • GitHub Check: build-and-tests (clang-16, none, debugoptimized, ubuntu-24.04, false)
  • GitHub Check: build-and-tests (clang-18, none, debugoptimized, ubuntu-24.04, false)
  • GitHub Check: build-and-tests (clang-15, none, debugoptimized, ubuntu-22.04, false)
  • GitHub Check: build-and-tests (gcc-14, address, debug, ubuntu-24.04, -Dfrr=enabled, true)
  • GitHub Check: build-and-tests (gcc-13, none, debugoptimized, ubuntu-24.04, -Dfrr=enabled, false)
  • GitHub Check: build-cross-aarch64
  • GitHub Check: rpm
  • GitHub Check: deb
🔇 Additional comments (7)
cli/main.c (1)

149-156: LGTM: Line buffering initialization is correct.

The explicit line buffering setup before locale initialization ensures consistent output behavior, with proper per-stream error handling.

main/dpdk.c (1)

65-65: Verify implications of routing all logs to stdout.

Moving error and warning logs from stderr to stdout may complicate error handling in scripts and pipelines that rely on stderr for filtering failures.

smoke/_init.sh (5)

104-104: Improved randomness generation.

Using /dev/urandom is more robust than shell built-ins. The static analysis warning about run_id being unused is likely a false positive if this variable is consumed elsewhere in the test suite.


131-136: LGTM: TTY-aware xtrace colorization.

The conditional setup colorizes bash tracing output only when stdout is a terminal, improving readability without breaking non-interactive usage.


147-154: LGTM: Conditional grout output colorization.

The tty-aware routing colors grout stdout (blue) and stderr (bold red) only when interactive, preserving plain output for automation.


169-174: LGTM: Consistent pattern for events colorization.

The yellow coloring for events output follows the same tty-aware pattern used throughout.


180-185: LGTM: Zebra log colorization follows established pattern.

The magenta coloring for zebra logs maintains consistency with the tty-aware colorization scheme.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@aharivel
Copy link
Collaborator

Tested-by: Anthony Harivel aharivel@redhat.com

Copy link
Collaborator

@aharivel aharivel left a comment

Choose a reason for hiding this comment

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

LGTM - It's good to see the life in color !

@christophefontaine christophefontaine merged commit dbb7622 into DPDK:main Oct 23, 2025
12 checks passed
@rjarry rjarry deleted the smoke-color branch October 23, 2025 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants