test: Phase 6 Prometheus monitoring test suite#1140
Conversation
TASK-057: Metrics endpoint validation (HTTP, format, 8 metrics, labels) TASK-058: Performance tests (interval, atomicity, response time) TASK-062: Persistence tests (volumes, jobs.log, histogram) TASK-063: Scaling tests (port mapping, isolation, multi-runner) TASK-067: Security tests (token leak scanning, safe labels) TASK-068: Documentation validation (file existence, permissions, syntax) TASK-069: Updated tests/README.md with Prometheus test documentation TASK-070: Added metrics tests to CI/CD pipeline integration matrix All tests run in dual mode: static analysis always passes in CI, runtime tests activate when containers are running. Issue: #1064
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive test suite for the Prometheus monitoring feature, marking the completion of Phase 6: Testing & Validation. It ensures the reliability, performance, and security of the metrics collection and exposure, while also updating the project plan and test documentation to reflect these advancements. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 an extensive and well-structured test suite for the Prometheus monitoring feature, adding 6 new test scripts that cover documentation, endpoint validation, performance, persistence, scaling, and security. The tests are thorough and greatly enhance the quality assurance for this feature. I've provided a few suggestions to improve the portability and efficiency of the test scripts, along with minor fixes for the documentation.
| ``` | ||
|
|
||
| ## 🚨 Issue Prevention | ||
| ## � Prometheus Metrics Tests |
There was a problem hiding this comment.
This header contains a replacement character (``), which indicates that the intended emoji is not rendering correctly. Using a standard, widely supported emoji will improve readability.
| ## � Prometheus Metrics Tests | |
| ## 🧪 Prometheus Metrics Tests |
| done | ||
| ``` | ||
|
|
||
| ## �🚨 Issue Prevention |
There was a problem hiding this comment.
This header contains a replacement character (``), which indicates that the intended emoji is not rendering correctly. Using a standard, widely supported emoji will improve readability.
| ## �🚨 Issue Prevention | |
| ## 🚨 Issue Prevention |
| fi | ||
|
|
||
| # YAML syntax check (basic — check for tab characters) | ||
| if grep -qP '\t' "$PROM_CONFIG" 2>/dev/null; then |
There was a problem hiding this comment.
The -P flag for grep is a GNU extension and is not available on all systems (e.g., macOS), which could affect developers running tests locally. For better portability, you can use a literal tab character inside the pattern, which is supported by all versions of grep.
| if grep -qP '\t' "$PROM_CONFIG" 2>/dev/null; then | |
| if grep -q '` `' "$PROM_CONFIG"; then |
| IFS=':' read -r compose_file expected_port <<< "$entry" | ||
| COMPOSE_PATH="$REPO_ROOT/$compose_file" | ||
| if [[ -f "$COMPOSE_PATH" ]]; then | ||
| if grep -q "${expected_port}:9091" "$COMPOSE_PATH" || grep -q "${expected_port}" "$COMPOSE_PATH"; then |
There was a problem hiding this comment.
The second condition grep -q "${expected_port}" is too broad and could match the port number in other contexts, leading to false positives. The first condition grep -q "${expected_port}:9091" is much more specific and seems sufficient. The test-metrics-scaling.sh script already uses this stricter check, and applying it here would improve consistency and robustness.
| if grep -q "${expected_port}:9091" "$COMPOSE_PATH" || grep -q "${expected_port}" "$COMPOSE_PATH"; then | |
| if grep -q "${expected_port}:9091" "$COMPOSE_PATH"; then |
| BAD_LINES=0 | ||
| while IFS= read -r line; do | ||
| FIELDS=$(echo "$line" | awk -F, '{print NF}') | ||
| if [[ "$FIELDS" -ne 5 ]]; then | ||
| BAD_LINES=$((BAD_LINES + 1)) | ||
| fi | ||
| done < "$MOCK_JOBS_LOG" |
There was a problem hiding this comment.
This while loop forks a new echo and awk process for every line in the log file, which can be inefficient for large files. You can achieve the same result more efficiently with a single awk command that processes the entire file at once.
| BAD_LINES=0 | |
| while IFS= read -r line; do | |
| FIELDS=$(echo "$line" | awk -F, '{print NF}') | |
| if [[ "$FIELDS" -ne 5 ]]; then | |
| BAD_LINES=$((BAD_LINES + 1)) | |
| fi | |
| done < "$MOCK_JOBS_LOG" | |
| BAD_LINES=$(awk -F, 'NF != 5 { count++ } END { print count+0 }' "$MOCK_JOBS_LOG") |
- Fix bash set -e incompatibility: replace ((PASS++)) with
PASS=$((PASS + 1)) in all 6 integration test scripts (((0++))
returns exit code 1 which triggers set -e abort)
- Fix GNU grep "Unmatched \{" error in security test by using
grep -vF for fixed string matching instead of regex
- Fix shellcheck SC2034 warnings in metrics-collector.sh by adding
function-level disable directive for nameref variables
- Fix shellcheck SC2004 style issues by removing unnecessary $ on
arithmetic variables in array indexing
- Add SKIP handling to test-metrics-phase1.sh test_result function
(was treating SKIP as FAIL)
1 participant
Summary
Implements Phase 6: Testing & Validation for the Prometheus monitoring feature (Issue #1064).
Type of Change
Related Issues
Changes Made
6 New Test Scripts (2,003 lines)
All tests operate in dual mode: static analysis always runs in CI (no containers needed), runtime tests activate when containers are available.
Updated Files
Backlogged Tasks (require running infrastructure)
TASK-059, 060, 061, 064, 065, 066 - load testing, Grafana validation, storage benchmarks
Testing
Checklist
/cc @copilot