test(smoke): enhance flow smoke test with comprehensive checks and Instatus incident management - #227
test(smoke): enhance flow smoke test with comprehensive checks and Instatus incident management#227rvignesh89 wants to merge 1 commit into
Conversation
…status incident management - Add parse_via_gpt_vision and speech_to_text validation steps - Make test time-independent by validating execution timestamps - Verify 7 flow responses including text, audio, and AI-powered checks - Fix timezone handling for consistent cross-environment execution - Implement incident-based Instatus integration that opens/resolves incidents instead of just updating component status, enabling Discord notifications - Add robust timestamp matching with ±1 minute tolerance for CI/server clock differences Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughThe smoke test now derives a UTC execution timestamp, searches for a matching completion marker within a one-minute tolerance, and validates the seven messages from that simulator run. Instatus reporting now lists named open incidents, creates an investigating incident on the first failure, suppresses duplicate failures, and resolves open incidents after a passing run using the supplied template. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cypress/e2e/smoke.spec.ts`:
- Around line 27-59: Move the Date.now()-based timestamp candidate and
finishedRe construction into a .then() chained after previewButton.click() or
the flow-triggering command. Keep the existing ±1-minute matching and
message-slice logic unchanged, but ensure the timestamp anchor is captured when
the flow starts rather than while Cypress is still queuing commands.
In `@cypress/plugins/instatus.ts`:
- Around line 46-60: Update createIncident and resolveIncident to validate each
fetch response with res.ok before logging success; throw an error containing the
operation and HTTP status when either request fails, so callers do not treat
unsuccessful lifecycle operations as completed. In resolveIncident, remove the
JSON Content-Type header or provide an appropriate request body, while
preserving the existing successful-request behavior.
- Around line 78-88: Update the reportToInstatus call in the Cypress
configuration example to pass resolveTemplateId as the fourth argument and
passed as the fifth argument, matching the function signature and preserving
correct status reporting for both passing and failing runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: aade0ac0-d4a7-4ad2-b5be-b075324ee3a0
📒 Files selected for processing (2)
cypress/e2e/smoke.spec.tscypress/plugins/instatus.ts
| const fmt = (d: Date): string => { | ||
| const p = (n: number): string => String(n).padStart(2, '0'); | ||
| return `${p(d.getUTCDate())}-${p(d.getUTCMonth() + 1)}-${d.getUTCFullYear()} ${p( | ||
| d.getUTCHours() | ||
| )}:${p(d.getUTCMinutes())}`; | ||
| }; | ||
| const now = Date.now(); | ||
| const candidates = [-60000, 0, 60000].map((offset) => fmt(new Date(now + offset))); | ||
| const escaped = candidates.map((c) => c.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')); | ||
| const finishedRe = new RegExp(`Test Finished - (${escaped.join('|')})`); | ||
|
|
||
| // The full flow is slow — wait (retrying) for THIS run's unique finish marker. | ||
| cy.contains('[data-testid="simulatorMessage"]', finishedRe, { timeout: 180000 }).should( | ||
| 'exist' | ||
| ); | ||
| cy.get('[data-testid="simulatedMessages"]') | ||
| .find('[data-testid="simulatorMessage"]') | ||
| .then(($messages) => { | ||
| const lastThree = $messages.slice(-3); | ||
| // Anchor on the last matching "Test Finished" bubble and take the 7 bubbles | ||
| // of that run. Bounding the slice with the same timestamp on both ends means | ||
| // a truncated/failed run (which jumps straight to "Test Finished") fails the | ||
| // "Test on" assertion instead of borrowing a prior run's bubbles. | ||
| let endIdx = -1; | ||
| let execTime = ''; | ||
| $messages.each((i, el) => { | ||
| const match = Cypress.$(el).text().match(finishedRe); | ||
| if (match) { | ||
| endIdx = i; | ||
| [, execTime] = match; | ||
| } | ||
| }); | ||
| expect(endIdx, 'index of the "Test Finished" message').to.be.gte(6); | ||
| const responses = $messages.slice(endIdx - 6, endIdx + 1); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd 'smoke.spec.ts' -x cat -n {}
fd 'cypress.config' -x cat -n {}Repository: glific/cypress-testing
Length of output: 7771
Anchor the timestamp after the flow starts
Date.now() is evaluated while Cypress is still queuing commands, so this anchor can drift well ahead of the actual previewButton.click()/flow start. Capture now and build finishedRe in a .then() chained off the trigger command; otherwise a slow run can push the real server minute outside the ±1 minute window and make the smoke test miss its own finish marker.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 35-35: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(Test Finished - (${escaped.join('|')}))
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cypress/e2e/smoke.spec.ts` around lines 27 - 59, Move the Date.now()-based
timestamp candidate and finishedRe construction into a .then() chained after
previewButton.click() or the flow-triggering command. Keep the existing
±1-minute matching and message-slice logic unchanged, but ensure the timestamp
anchor is captured when the flow starts rather than while Cypress is still
queuing commands.
| async function createIncident(apiKey: string, pageId: string, componentId: string): Promise<void> { | ||
| const res = await fetch(`${API_BASE}/${pageId}/incidents`, { | ||
| method: 'POST', | ||
| headers: authHeaders(apiKey), | ||
| body: JSON.stringify({ | ||
| name: INCIDENT_NAME, | ||
| message: 'The automated Glific flow smoke test failed.', | ||
| status: 'INVESTIGATING', | ||
| notify: true, | ||
| components: [componentId], | ||
| statuses: [{ id: componentId, status: 'MAJOROUTAGE' }], | ||
| }), | ||
| }); | ||
| console.log(`Instatus: opened incident (notify) → HTTP ${res.status}`); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
createIncident and resolveIncident don't check res.ok — silent failures break the incident lifecycle.
Both functions log the HTTP status but never check whether the response succeeded. This causes two problems:
-
createIncident(lines 59): Logs"opened incident"even on a 4xx/5xx response. If creation fails, the next run'sgetOpenIncidentsreturns empty (incident was never created), so it tries to create again — re-notifying Discord on every failing run, defeating the "one incident per outage" design. -
resolveIncident(line 75): Logs"resolved incident"even on failure. If resolution fails, the incident stays open indefinitely and the error is only visible in CI logs.
Additionally, resolveIncident sends a POST with Content-Type: application/json but no body. Some API gateways reject this combination with a 400.
[major_severity]
🔧 Proposed fix: add `res.ok` checks and throw on failure
async function createIncident(apiKey: string, pageId: string, componentId: string): Promise<void> {
const res = await fetch(`${API_BASE}/${pageId}/incidents`, {
method: 'POST',
headers: authHeaders(apiKey),
body: JSON.stringify({
name: INCIDENT_NAME,
message: 'The automated Glific flow smoke test failed.',
status: 'INVESTIGATING',
notify: true,
components: [componentId],
statuses: [{ id: componentId, status: 'MAJOROUTAGE' }],
}),
});
- console.log(`Instatus: opened incident (notify) → HTTP ${res.status}`);
+ if (!res.ok) {
+ throw new Error(`Instatus: failed to open incident → HTTP ${res.status}`);
+ }
+ console.log(`Instatus: opened incident (notify) → HTTP ${res.status}`);
} async function resolveIncident(
apiKey: string,
pageId: string,
incidentId: string,
resolveTemplateId: string
): Promise<void> {
const res = await fetch(
`${API_BASE_V2}/${pageId}/incidents/${incidentId}/incident-updates/${resolveTemplateId}`,
{
method: 'POST',
headers: authHeaders(apiKey),
+ body: JSON.stringify({}),
}
);
- console.log(`Instatus: resolved incident ${incidentId} (notify) → HTTP ${res.status}`);
+ if (!res.ok) {
+ throw new Error(`Instatus: failed to resolve incident ${incidentId} → HTTP ${res.status}`);
+ }
+ console.log(`Instatus: resolved incident ${incidentId} (notify) → HTTP ${res.status}`);
}Also applies to: 62-76
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cypress/plugins/instatus.ts` around lines 46 - 60, Update createIncident and
resolveIncident to validate each fetch response with res.ok before logging
success; throw an error containing the operation and HTTP status when either
request fails, so callers do not treat unsuccessful lifecycle operations as
completed. In resolveIncident, remove the JSON Content-Type header or provide an
appropriate request body, while preserving the existing successful-request
behavior.
| export async function reportToInstatus( | ||
| apiKey: string, | ||
| pageId: string, | ||
| componentId: string, | ||
| resolveTemplateId: string, | ||
| passed: boolean | ||
| ): Promise<null> { | ||
| if (!apiKey || !pageId || !componentId) { | ||
| if (!apiKey || !pageId || !componentId || !resolveTemplateId) { | ||
| console.warn('Instatus credentials not set — skipping status report'); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the actual cypress.config.ts passes resolveTemplateId to reportToInstatus.
# If only cypress.config.ts.example exists, the contract is broken.
# Find the actual config file (not the .example)
fd -t f 'cypress.config.ts' --exclude '*.example' .
# If found, check how reportToInstatus is called
fd -t f 'cypress.config.ts' --exclude '*.example' . -x sed -n '1,80p' {} | grep -A 10 'reportToInstatus'
# Also check for INSTATUS_RESOLVE_TEMPLATE_ID in env/config
rg -n 'INSTATUS_RESOLVE_TEMPLATE_ID|resolveTemplateId' --type tsRepository: glific/cypress-testing
Length of output: 160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Files =="
git ls-files | rg '(^|/)cypress\.config\.ts$|(^|/)cypress\.config\.ts\.example$|(^|/)instatus\.ts$'
echo
echo "== reportToInstatus references =="
rg -n 'reportToInstatus|INSTATUS_RESOLVE_TEMPLATE_ID|resolveTemplateId' .
echo
echo "== cypress.plugins.instatus.ts (relevant lines) =="
sed -n '1,220p' cypress/plugins/instatus.tsRepository: glific/cypress-testing
Length of output: 5108
Pass resolveTemplateId from cypress.config.ts.example.
cypress.config.ts.example:39 still calls reportToInstatus(...) with four args, so passed shifts into resolveTemplateId and the real passed flag becomes undefined. Passing runs will open incidents, and failing runs will skip reporting.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cypress/plugins/instatus.ts` around lines 78 - 88, Update the
reportToInstatus call in the Cypress configuration example to pass
resolveTemplateId as the fourth argument and passed as the fifth argument,
matching the function signature and preserving correct status reporting for both
passing and failing runs.
1 participant
Summary
Summary by CodeRabbit
Bug Fixes
Monitoring