Skip to content

feat(groq): Displays an ASCII bar graph of disk usage per mounted filesystem. - #5820

Open
polsala wants to merge 1 commit into
mainfrom
ai/groq-20260901-1124
Open

feat(groq): Displays an ASCII bar graph of disk usage per mounted filesystem.#5820
polsala wants to merge 1 commit into
mainfrom
ai/groq-20260901-1124

Conversation

@polsala

@polsala polsala commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Implementation Summary

  • Utility: nightly-disk-usage-visualizer
  • Provider: groq
  • Location: bash-utils/nightly-nightly-disk-usage-visualize-2
  • Files Created: 3
  • Description: Displays an ASCII bar graph of disk usage per mounted filesystem.

Rationale

  • Automated proposal from the Groq generator delivering a fresh community utility.
  • This utility was generated using the groq AI provider.

Why safe to merge

  • Utility is isolated to bash-utils/nightly-nightly-disk-usage-visualize-2.
  • README + tests ship together (see folder contents).
  • No secrets or credentials touched.
  • All changes are additive and self-contained.

Test Plan

  • Follow the instructions in the generated README at bash-utils/nightly-nightly-disk-usage-visualize-2/README.md
  • Run tests located in bash-utils/nightly-nightly-disk-usage-visualize-2/tests/

Links

  • Generated docs and examples committed alongside this change.

Mock Justification

  • Not applicable; generator did not introduce new mocks.

@polsala

polsala commented Sep 1, 2026

Copy link
Copy Markdown
Owner Author

🤖 Review by GROQ Agent

✅ What's solid

  • Self‑contained utility – the script lives in its own directory, has no external runtime dependencies beyond standard Unix tools (bash, df, awk, seq), and does not touch any secrets or system state.
  • Robust Bash optionsset -euo pipefail is enabled, which prevents silent failures and accidental use of undefined variables.
  • Clear output format – the 20‑character bar graph is easy to read, and the printf formatting aligns columns nicely.
  • README & test bundled – the PR ships a minimal usage guide and a functional test that exercises the core logic with a static df dump.
  • Error handling for missing input file – the script exits with a helpful message when a supplied file does not exist.

🧪 Tests

  • Positive coverage – the test validates the exact expected output for a typical df snapshot (including a tmpfs line).
  • Suggested enhancements
    • Use a temporary file instead of a hard‑coded /tmp/df_test.txt so the test does not leave stray files on the host and works in parallel CI runs:

      tmpfile=$(mktemp)
      cat > "$tmpfile" <<'EOF'
      ...
      EOF
      output=$("$SCRIPT" "$tmpfile")
      rm -f "$tmpfile"
    • Add edge‑case scenarios:

      • 0 % usage (should render an empty bar)
      • 100 % usage (full bar)
      • A line with missing fields or malformed % value (ensure the script fails gracefully).
    • Test error paths: invoke the script with a non‑existent file and assert that it exits with a non‑zero status and prints the expected error message.

    • Avoid brittle string comparison – use diff -u or cmp to compare multi‑line output, which makes whitespace differences easier to spot:

      diff -u <(echo "$output") <(cat <<'EOT'
      /                     45% [#########-----------]
      /run                  10% [##------------------]
      /data                 90% [##################--]
      EOT
      ) || { echo "Test failed"; exit 1; }

🔒 Security

  • Input handling – the script only reads a file path supplied as an argument and never executes its contents, which is safe.

  • Quoting – all variable expansions used in printf are quoted, preventing word‑splitting or globbing attacks.

  • Potential improvement: avoid loading the entire file into a variable (DF_DATA=$(cat "$DF_INPUT")). Streaming the file directly into the while read loop reduces memory footprint and eliminates a needless subshell:

    if [[ $# -gt 0 ]]; then
        DF_INPUT="$1"
        [[ -f "$DF_INPUT" ]] || { echo "File not found: $DF_INPUT" >&2; exit 1; }
        exec 3<"$DF_INPUT"
    else
        exec 3< <(df -h --output=source,size,used,avail,pcent,target -x tmpfs -x devtmpfs)
    fi
    
    while IFS= read -r line <&3; do
        ...
    done
    exec 3<&-
  • Portability noteseq is used to generate the bar characters. While common on GNU/Linux, some minimal containers (e.g., BusyBox) may lack it. A pure‑bash alternative is:

    bar=$(printf '#%.0s' $(seq 1 $bar_len))
    empty=$(printf '-%.0s' $(seq 1 $((20 - bar_len))))

    or even a loop with printf if seq is unavailable.

🧩 Docs/DX

  • README depth – the current README is a single paragraph. Consider expanding it with:

    • Synopsis (one‑liner usage, e.g., disk-usage.sh [FILE]).
    • Options (explain that a positional argument is a pre‑captured df dump).
    • Exit codes (0 on success, non‑zero on error).
    • Examples showing both live and file‑based runs, plus a sample pipeline (df -h | ./disk-usage.sh).
    • Dependencies (list required commands: bash, df, awk, seq).
    • Contribution guidelines (how to add new tests, linting, etc.).
  • File naming – the directory nightly-nightly-disk-usage-visualize-2 contains a duplicated “nightly”. A clearer name such as bash-utils/disk-usage-visualizer would improve discoverability.

  • License header – adding a short SPDX identifier (e.g., # SPDX-License-Identifier: MIT) at the top of the script helps downstream tooling.

🧱 Mocks/Fakes

  • Current approach – the test supplies a static df dump, which is an effective mock for the script’s input.

  • Potential refinement – abstract the data‑source into a function (e.g., get_df_data) that defaults to df -h … but can be overridden in tests. This would let the test inject data without needing a temporary file:

    # In disk-usage.sh
    get_df_data() { df -h --output=...; }
    
    # In test
    source "$SCRIPT_DIR/disk-usage.sh"
    get_df_data() { cat "$tmpfile"; }
    output=$(disk_usage_main)

    This pattern makes unit‑testing easier and eliminates reliance on external commands during CI.


Overall, the utility is functional and well‑isolated. Strengthening the test suite, tightening a few Bash idioms, and enriching the documentation will make the contribution more robust and maintainable.

@polsala

polsala commented Sep 1, 2026

Copy link
Copy Markdown
Owner Author

🤖 Review by GEMINI Agent

✅ What's solid

  • Self-contained utility: The script operates independently, relying only on standard Unix tools (bash, df, awk, seq). It does not introduce external runtime dependencies or modify system state, making it low-impact and easy to integrate.
  • Robust Bash options: The inclusion of set -euo pipefail in both the main script and the test script significantly enhances reliability by preventing silent failures, ensuring variables are defined, and handling pipeline errors gracefully.
  • Clear input handling: The script intelligently distinguishes between processing live df output and reading from a provided file. This design choice not only makes the utility flexible for various use cases but also greatly simplifies testing.
  • Specific df command: Utilizing df -h --output=source,size,used,avail,pcent,target -x tmpfs -x devtmpfs ensures consistent parsing by specifying column order and filters out common, less relevant filesystems, improving the clarity of the output.
  • Efficient bar generation: The method of generating the ASCII bar using printf "%0.s#" and seq is an idiomatic and efficient Bash technique for repeating characters, contributing to the script's performance.

🧪 Tests

  • Limited test coverage: The current test_disk_usage.sh provides a good foundational test for a single happy path. To enhance robustness, consider expanding the test suite to cover edge cases and potential variations in df output.
    • Add tests for filesystems with 0% usage and 100% usage to ensure the bar rendering is accurate at boundaries.
    • Include scenarios with empty df output or df output containing only the header to verify graceful handling.
    • Test with df output that might have very long mount point names to check output formatting and truncation (if desired).
  • Temporary file cleanup: The test script creates /tmp/df_test.txt but does not explicitly remove it upon completion or failure. Implement a trap to ensure temporary files are cleaned up reliably.
    cleanup() {
      rm -f /tmp/df_test.txt
    }
    trap cleanup EXIT
  • Test assertion clarity: While functional, the if [[ "$output" != "$expected" ]]; then block could benefit from a more structured test framework (e.g., bats, shunit2) for larger test suites, though for a single script, the current approach is acceptable.

🔒 Security

  • Read-only operation: The script's function is purely informational, reading system data (df output) without performing any write operations or modifying system state. This inherently minimizes security risks.
  • Reliance on trusted utilities: The script's dependencies are limited to core Unix utilities, which are generally well-vetted and secure. This reduces the attack surface associated with third-party libraries.
  • Input processing: When processing a user-provided file, the script pipes its content to awk. While awk '{print $N}' is generally safe against command injection, ensure that the context of how this utility might be used (e.g., with untrusted input files) is considered. For typical df output, this is not a concern.

🧩 Docs/DX

  • Naming inconsistency: The PR body refers to the utility as nightly-disk-usage-visualizer, but the directory structure is bash-utils/nightly-nightly-disk-usage-visualize-2. The double "nightly" in the directory name appears to be a typo and creates an inconsistency that could lead to confusion. Standardize the naming for clarity.
  • Magic number for bar width: The bar length of 20 characters is a "magic number" embedded directly in the code. Define this as a constant at the top of the script to improve readability and maintainability, making it easier to adjust the bar width in the future.
    BAR_WIDTH=20
    # ...
    bar_len=$(( (percent * BAR_WIDTH) / 100 ))
    empty=$(printf "%0.s-" $(seq 1 $((BAR_WIDTH - bar_len))))
  • Error handling for df command: If the df command itself fails (e.g., due to an invalid option in a different environment or a system issue), the script might exit abruptly due to set -e. Consider adding explicit error handling for the df command execution to provide more user-friendly feedback.
    DF_DATA=$(df -h --output=source,size,used,avail,pcent,target -x tmpfs -x devtmpfs 2>/dev/null || { echo "Error: Failed to get disk usage data from 'df'." >&2; exit 1; })
  • awk call optimization: The script currently calls awk three times within the loop for each line of df output. While acceptable for typical df output sizes, for very large outputs, this could be optimized by performing a single awk call to extract all necessary fields per line, or by using Bash's built-in string manipulation if performance becomes a critical factor.
    # Example of single awk call per line
    # while IFS= read -r line; do
    #   [[ -z "$line" ]] && continue
    #   if [[ "$line" == *"Filesystem"* ]]; then
    #     continue
    #   fi
    #   read -r source pcent mount <<< $(echo "$line" | awk '{print $1, $5, $6}')
    #   percent=${pcent%\%}
    #   # ... rest of the logic
    # done <<< "$DF_DATA"

🧱 Mocks/Fakes

  • Effective mocking strategy: The use of a temporary file (/tmp/df_test.txt) to provide mocked df output is an excellent and robust strategy for testing shell scripts. It ensures deterministic test results, isolates the script from live system changes, and allows for easy creation of various test scenarios.
  • Clear mock data: The cat > /tmp/df_test.txt <<'EOF' and read -r -d '' expected <<'EOT' constructs are clear and readable ways to define mock input and expected output, making the test script easy to understand and maintain.
  • Appropriate scope: The PR correctly notes that no new mocks were introduced beyond what is necessary for testing this specific utility, which aligns with best practices for self-contained components.

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.

1 participant