Add performance tests

[alexeyr-ci2]

Summary

Add performance tests. Fixes #1777.

Pull Request checklist

• [ ] Add/update test to cover these changes
• Update documentation
• Update CHANGELOG file

---

This change is 

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch alexeyr/performance-tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

Code Review - PR #1868: Add Performance Tests

Thanks for this contribution! This is a solid implementation of performance testing infrastructure.

---

✅ Strengths

1. Comprehensive Tool Coverage: Using three industry-standard tools (Fortio, Vegeta, k6) provides good cross-validation
2. Well-Structured Workflow: Clear separation of concerns with logical step grouping
3. Good Error Handling: Proper use of set -euo pipefail and validation steps
4. Caching Strategy: Smart use of GitHub Actions cache for benchmark tool binaries
5. Security Conscious: SSH access properly gated with warnings and actor-only access
6. Flexible Configuration: Parameterized inputs allow customization

---

🐛 Critical Issues

1. Node.js Version Mismatch (benchmark.yml:155)

   • Uses Node 22, but PR Fix Node.js 22 V8 cache crash by reverting to Node 20 #1925 documents Node 22 V8 cache crashes
   • Recommend changing to Node 20 until Fix Node.js 22 V8 cache crash by reverting to Node 20 #1925 is resolved

2. Missing Trailing Newlines (CRITICAL per CLAUDE.md)

   • Files spec/dummy/bin/prod and spec/dummy/bin/prod-assets missing final newlines
   • This will cause CI failures - Run: rake autofix to fix

3. Race Condition in Server Startup (benchmark.yml:220)

   • Background server might not be fully initialized when curl check succeeds
   • Recommend adding health check endpoint or verifying multiple successful responses

4. Incomplete PR Checklist

   • Documentation and CHANGELOG boxes unchecked
   • Per CLAUDE.md: Should update CHANGELOG for user-visible features

---

⚠️ Performance & Quality Issues

5. bench.sh:66 - Unrealistic iteration count assumes 10 iterations/sec per VU
6. Fortio Optimization Missing - Referenced guide not implemented
7. No Resource Monitoring - Missing CPU/memory metrics
8. Missing Test Validation - No thresholds to detect performance regressions
9. Single Route Testing - Only tests server_side_hello_world_hooks
10. Complex Validation Logic - Lines 17-27 hard to read
11. Inconsistent Quoting - Variables not consistently quoted
12. Hard-coded Timeouts - 30s and 60s timeouts not configurable

---

📚 Documentation Issues

13. Missing Documentation - No README for running benchmarks locally
14. Workflow Triggers - Runs on ALL PRs (expensive, should use labels/manual only)
15. No Historical Tracking - Results not compared over time

---

🎯 Before Merging

1. Fix Node.js version (22 → 20)
2. Add trailing newlines
3. Run bundle exec rubocop
4. Update CHANGELOG.md
5. Add performance thresholds
6. Update workflow triggers

---

📊 Summary

Assessment: Good foundation but needs critical fixes before merge

Effort: Critical issues ~30 mins, High priority ~1-2 hours

Main concerns: Node.js version mismatch and missing trailing newlines will cause CI failures.

Let me know if you need help with any of these!

[claude]

PR Review: Add Performance Tests (1868)

Summary

This PR adds comprehensive performance testing infrastructure using three industry-standard benchmarking tools (Fortio, Vegeta, and k6). Well-structured implementation addressing issue 1777.

Critical Issues

1. Missing Trailing Newlines

CRITICAL per CLAUDE.md: Files MUST end with newline character or CI will fail:

• spec/dummy/bin/prod (line 4)
• spec/dummy/bin/prod-assets (line 9)
• spec/performance/bench.sh (line 202)

2. RuboCop Required

CRITICAL per CLAUDE.md: Must run bundle exec rubocop and fix ALL violations before commit.

Potential Bugs

K6 JSON Parsing (bench.sh:187-192)

The jq expression uses add which fails on empty arrays. Use add // 0 for safety.

Server Startup Race (benchmark.yml:254)

Uses exit 0 inside loop which exits the entire step. Should use break instead.

npm vs yarn (benchmark.yml:200)

Uses npm install --global yalc but CLAUDE.md requires yarn-only commands.

Code Quality Strengths

1. Excellent tool selection - three benchmarking tools for cross-validation
2. Well-structured workflow with clear sections
3. Good error handling with set -euo pipefail
4. Comprehensive parameterization via workflow inputs
5. Smart binary caching for faster CI
6. Security-conscious SSH access gating

Suggestions

1. Shell portability: Replace bc with awk for float comparisons
2. Hardcoded route: Make server_side_hello_world_hooks configurable
3. Duration: 30s may be too short, consider 60s for stable measurements
4. Add documentation for local benchmark execution
5. Consider performance regression detection vs master
6. Add spec/performance/README.md with interpretation guide

Test Coverage

Good: Infrastructure in place, runs on PR/push, results preserved as artifacts
Missing: No bash script tests, no performance thresholds, no local testing docs

Verdict

Solid PR with high code quality. Performance testing infrastructure will be valuable.

Required Before Merge:

1. Fix trailing newlines (will cause CI failure)
2. Run bundle exec rubocop and fix violations
3. Fix K6 empty array bug

Recommended:

1. Add basic documentation
2. Verify workflow passes on CI
3. Consider CHANGELOG entry

Great work overall!