test: Performance tests suite #21226
Conversation
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
There was a problem hiding this comment.
mrge reviewed this PR and found no issues. Review PR in mrge.io.
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (05/10/25)1 reviewer was added to this PR based on Keith Williams's automation. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
| } | ||
|
|
||
| export function randomSleep(min = SLEEP_DURATION.SHORT, max = SLEEP_DURATION.MEDIUM) { | ||
| const sleepTime = Math.random() * (max - min) + min; |
There was a problem hiding this comment.
Missing validation that min is less than max
tests/performance/utils/config.js
Outdated
| } | ||
|
|
||
| export function randomQueryParam() { | ||
| return `nocache=${new Date().getTime()}`; |
There was a problem hiding this comment.
Using Date.now() would be more efficient than new Date().getTime()
| }, | ||
| }; | ||
|
|
||
| export const TEST_USERS = new SharedArray("users", function () { |
There was a problem hiding this comment.
Hardcoded test credentials in source code
| import { viewBookingPage } from "../utils/helpers.js"; | ||
|
|
||
| export const options = { | ||
| stages: [ |
There was a problem hiding this comment.
The spike test has a very aggressive ramp-up (1m to reach 5000 VUs) which may not accurately simulate real-world traffic patterns and could overwhelm the system too quickly.
tests/performance/spike/booking.js
Outdated
| export default function () { | ||
| viewBookingPage("pro", "30min"); | ||
|
|
||
| sleep(0.01); |
There was a problem hiding this comment.
Sleep duration is too short for a spike test. The 0.01s sleep time may not provide enough pause between iterations, potentially causing excessive load beyond what's intended.
| import { sleep } from "k6"; | ||
|
|
||
| import { THRESHOLDS } from "../utils/config.js"; | ||
| import { viewBookingPage } from "../utils/helpers.js"; |
There was a problem hiding this comment.
Importing from a file that doesn't appear to exist in the repository
| @@ -0,0 +1,26 @@ | |||
| import { sleep } from "k6"; | |||
|
|
|||
| import { THRESHOLDS } from "../utils/config.js"; | |||
There was a problem hiding this comment.
Importing from a file that doesn't appear to exist in the repository
| return group("View Booking Page", () => { | ||
| const url = `${BASE_URL}/${username}/${eventSlug}?${randomQueryParam()}`; | ||
|
|
||
| const response = http.get(url, { |
There was a problem hiding this comment.
No timeout specified for HTTP request
tests/performance/utils/helpers.js
Outdated
|
|
||
| check(response, { | ||
| "Booking page loaded": (r) => r.status === 200, | ||
| "Has booking form": (r) => r.body.includes('data-testid="day"'), |
There was a problem hiding this comment.
Brittle test that relies on specific DOM element
|
This PR is being marked as stale due to inactivity. |
|
This PR is being marked as stale due to inactivity. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughA new performance testing framework using Grafana k6 was added for the booking flow. A GitHub Actions workflow (.github/workflows/performance-tests.yml) runs tests on release creation or manual dispatch. The repository now includes categorized k6 test scripts for smoke, load, stress, and spike scenarios, shared configuration and helper modules (base URL, thresholds, test users, sleep utilities, viewBookingPage helper), and a README describing setup, execution, and how to extend the suite. Each test simulates users viewing a booking page with varying VU profiles and thresholds. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
⏰ 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). (1)
✨ Finishing Touches
🧪 Generate unit 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
✅ No security or compliance issues detected. Reviewed everything up to 8ed0508. Security Overview
Detected Code Changes
Reply to this PR with |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (7)
tests/performance/utils/config.js (3)
35-40: Hardcoded test credentials pose a security risk.Test credentials are hardcoded in source code, which is a security vulnerability. Consider using environment variables or a secure configuration file.
47-47: Use Date.now() for better performance.
Date.now()is more efficient thannew Date().getTime()for getting timestamps.
50-53: Add validation for min/max parameters.The function doesn't validate that
minis less thanmax, which could lead to unexpected behavior.tests/performance/utils/helpers.js (2)
10-12: Specify timeout for HTTP requests.HTTP requests should have explicit timeout configurations to prevent hanging requests during performance testing.
16-16: Avoid brittle DOM element checks.The test relies on a specific DOM element (
data-testid="day"), making it fragile to UI changes. Consider checking for more stable indicators of successful page load.tests/performance/spike/booking.js (2)
7-12: Spike test ramp-up pattern may be too aggressive for realistic simulation.The current spike test rapidly increases from 500 to 5000 VUs in just 2 minutes, which may not accurately represent real-world traffic spike patterns and could overwhelm the system too quickly to gather meaningful performance data.
23-23: Sleep duration is too short for realistic load simulation.The 0.01 second sleep time may not provide sufficient pause between iterations, potentially causing excessive load beyond what's intended and not reflecting realistic user behavior patterns.
🧹 Nitpick comments (3)
tests/performance/load/booking.js (1)
23-23: Consider increasing sleep duration for more realistic load simulation.The 0.1 second sleep might be too aggressive and not representative of real user behavior. Consider increasing to 0.5-1 second for more realistic think time.
- sleep(0.1); + sleep(0.5);tests/performance/README.md (1)
62-62: Consider expanding the test scenarios section.The documentation mentions that the test suite "focuses specifically on the booking flow" but only describes one scenario. Consider adding more details about what aspects of the booking flow are being tested or potential future scenarios.
tests/performance/stress/booking.js (1)
25-25: Consider making sleep duration configurable.The sleep duration could be made configurable through the shared config to maintain consistency across different test types and allow for easy adjustment.
You could add a sleep configuration to
utils/config.js:export const SLEEP_DURATION = { SMOKE: 0.1, LOAD: 0.08, STRESS: 0.05, SPIKE: 0.01, };Then import and use it in the test:
+import { THRESHOLDS, SLEEP_DURATION } from "../utils/config.js"; - sleep(0.05); + sleep(SLEEP_DURATION.STRESS);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/performance-tests.yml(1 hunks)tests/performance/README.md(1 hunks)tests/performance/load/booking.js(1 hunks)tests/performance/smoke/booking.js(1 hunks)tests/performance/spike/booking.js(1 hunks)tests/performance/stress/booking.js(1 hunks)tests/performance/utils/config.js(1 hunks)tests/performance/utils/helpers.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
tests/performance/utils/helpers.js (1)
tests/performance/utils/config.js (2)
randomQueryParam(46-48)randomSleep(50-53)
tests/performance/load/booking.js (5)
tests/performance/smoke/booking.js (2)
options(6-13)options(6-13)tests/performance/stress/booking.js (2)
options(6-20)options(6-20)tests/performance/spike/booking.js (2)
options(6-18)options(6-18)tests/performance/utils/config.js (2)
THRESHOLDS(12-33)THRESHOLDS(12-33)tests/performance/utils/helpers.js (1)
viewBookingPage(6-22)
tests/performance/stress/booking.js (5)
tests/performance/load/booking.js (2)
options(6-18)options(6-18)tests/performance/smoke/booking.js (2)
options(6-13)options(6-13)tests/performance/spike/booking.js (2)
options(6-18)options(6-18)tests/performance/utils/config.js (2)
THRESHOLDS(12-33)THRESHOLDS(12-33)tests/performance/utils/helpers.js (1)
viewBookingPage(6-22)
⏰ 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). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Security Check
🔇 Additional comments (8)
tests/performance/smoke/booking.js (1)
1-19: LGTM! Well-structured smoke test configuration.The smoke test configuration is appropriate with 10 VUs for 2 minutes and uses proper thresholds. The test function correctly simulates user behavior with reasonable pacing.
tests/performance/load/booking.js (1)
6-18: LGTM! Well-designed load test stages.The staged load profile effectively ramps up to 2000 VUs with proper warm-up and cool-down phases. The threshold configuration is appropriate for load testing.
tests/performance/README.md (1)
68-70: All VU counts in README.md match test configurationsThe documented virtual user counts in tests/performance/README.md (lines 68–70) have been verified against their respective test scripts and accurately reflect the peak targets:
- Load Tests: 2,000 VUs
- Stress Tests: 4,000 VUs
- Spike Tests: 5,000 VUs
No updates required.
tests/performance/spike/booking.js (2)
1-4: LGTM: Imports are correctly structured.The imports follow k6 best practices and correctly reference the shared utilities for configuration and helper functions.
14-18: LGTM: Thresholds configuration is appropriate.The threshold configuration correctly uses the SPIKE-specific thresholds from the shared config, which are appropriately more lenient for spike testing scenarios.
tests/performance/stress/booking.js (3)
3-4: Imports are now valid with the addition of utility files.The previous concerns about importing from non-existent files are resolved as the utility files are being added in this PR.
6-20: LGTM: Stress test configuration is well-designed.The staged load pattern provides a realistic stress testing scenario with gradual ramp-up to 4000 VUs, appropriate hold periods, and proper threshold configuration for stress testing conditions.
22-26: LGTM: Test function implementation is consistent.The test function correctly uses the shared helper function and has a reasonable sleep duration (0.05s) that provides better pacing compared to the spike test.
- Add validation to randomSleep function to ensure min <= max - Replace new Date().getTime() with Date.now() for better performance - Move hardcoded test credentials to environment variables - Add HTTP timeout to requests in helpers.js - Improve DOM element checking to be less brittle - Fix spike test sleep duration from 0.01s to 0.1s - Make GitHub workflow BASE_URL configurable with inputs - Fix README filename reference from booking_flow.js to booking.js - Add documentation for new environment variables Co-Authored-By: anik@cal.com <adhabal2002@gmail.com>
What does this PR do?
Summary by mrge
Added performance tests suite using k6