Replace sleep in webhook tests with assertEventually by vermakhushboo · Pull Request #10656 · appwrite/appwrite

vermakhushboo · 2025-10-16T08:54:12Z

What does this PR do?

Replace sleep in webhook tests with assertEventually

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)

Related PRs and Issues

(Related PR or issue)

Checklist

Have you read the Contributing Guidelines on issues?
If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

coderabbitai · 2025-10-16T08:54:47Z

📝 Walkthrough

Walkthrough

Replaces fixed sleep-based waits in webhook end-to-end tests with polling using assertEventually (15s timeout, 500ms interval). Each polling block retrieves and asserts the last webhook request, recomputes and verifies the webhook signature, and validates common headers plus expanded X-Appwrite-Webhook-Events patterns for granular database/table/bucket/team CRUD paths. WebhooksCustomServerTest now imports and uses the Async trait. Several webhook auto-disable and failure-verification flows were updated to perform GETs inside assertEventually instead of after fixed sleeps.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Changes are widespread across tests with repeated, similar refactors (sleep → assertEventually) but include added event-pattern assertions and a trait addition, requiring moderate review to verify correctness and coverage.

Possibly related PRs

tests: make usagetests robust #9956 — Similar refactor converting fixed sleep waits into assertEventually polling in end-to-end tests.

Suggested reviewers

christyjacob4

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The PR title "Replace sleep in webhook tests with assertEventually" is directly aligned with the primary objective of the changeset. The raw summary confirms that the main change across both modified webhook test files involves replacing fixed sleep waits with asynchronous polling using the assertEventually method. The title is concise, single-sentence, and clearly conveys the essential change without unnecessary noise or vague terminology.
Description Check	✅ Passed	The PR description states "Replace sleep in webhook tests with assertEventually" which directly corresponds to the actual changes in the modified files. While the description is minimal and the template's test plan section is empty, it clearly articulates what the PR does and is related to the changeset. The description is not off-topic or completely unrelated; it accurately reflects the primary change being made across the webhook test files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feat-replace-sleep-in-webhooks-service

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f0660f3 and be983eb.

📒 Files selected for processing (1)

tests/e2e/Services/Webhooks/WebhooksBase.php (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

tests/e2e/Services/Webhooks/WebhooksBase.php (2)

tests/e2e/Scopes/Scope.php (2)

getLastRequest (130-140)

getRoot (160-202)

tests/e2e/Client.php (1)

Client (8-342)

⏰ 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: E2E Service Test (Dev Keys)
GitHub Check: Benchmark

🔇 Additional comments (3)

tests/e2e/Services/Webhooks/WebhooksBase.php (3)

150-168: LGTM! Race condition properly fixed.

The signature computation and validation are now correctly placed inside the assertEventually block, ensuring all assertions operate on the same webhook instance. This properly addresses the race condition flagged in previous reviews.

501-519: LGTM! Race condition properly fixed.

The signature computation and validation are now correctly placed inside the assertEventually block, consistent with the fix applied in testCreateAttributes. This eliminates the race condition where different webhook instances could be validated.

1490-1501: LGTM! Proper polling-based verification.

The webhook auto-disable verification is now correctly wrapped in assertEventually, replacing the fixed sleep approach with polling. This ensures the test waits for the webhook state to be updated after the failure events.

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

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

github-actions · 2025-10-16T08:56:51Z

Security Scan Results for PR

Docker Image Scan Results

Package	Version	Vulnerability	Severity
binutils	2.44-r2	CVE-2025-5244	HIGH
binutils	2.44-r2	CVE-2025-5245	HIGH
libxml2	2.13.8-r0	CVE-2025-49794	CRITICAL
libxml2	2.13.8-r0	CVE-2025-49796	CRITICAL
libxml2	2.13.8-r0	CVE-2025-49795	HIGH
libxml2	2.13.8-r0	CVE-2025-6021	HIGH
golang.org/x/crypto	v0.31.0	CVE-2025-22869	HIGH
golang.org/x/oauth2	v0.24.0	CVE-2025-22868	HIGH
stdlib	1.22.10	CVE-2025-47907	HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

coderabbitai

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2e10cd0 and f0660f3.

📒 Files selected for processing (2)

tests/e2e/Services/Webhooks/WebhooksBase.php (3 hunks)
tests/e2e/Services/Webhooks/WebhooksCustomServerTest.php (6 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

tests/e2e/Services/Webhooks/WebhooksBase.php (2)

tests/e2e/Scopes/Scope.php (1)

getLastRequest (130-140)

tests/e2e/Client.php (1)

Client (8-342)

tests/e2e/Services/Webhooks/WebhooksCustomServerTest.php (2)

tests/e2e/Scopes/Scope.php (1)

getLastRequest (130-140)

tests/e2e/Services/Webhooks/WebhooksBase.php (1)

getWebhookSignature (32-37)

⏰ 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). (3)

GitHub Check: E2E General Test
GitHub Check: Unit Test
GitHub Check: scan

🔇 Additional comments (6)

tests/e2e/Services/Webhooks/WebhooksBase.php (1)

1494-1505: LGTM!

The assertEventually usage here is correct. It polls the webhook configuration endpoint to verify that the webhook gets auto-disabled after 10 consecutive failures. Unlike the attribute/column creation tests, there's no race condition here since it's checking the webhook configuration state, not webhook events.

tests/e2e/Services/Webhooks/WebhooksCustomServerTest.php (5)

5-5: LGTM!

Adding the Async trait enables the assertEventually() method for asynchronous polling throughout the test class. This is the correct approach for replacing fixed sleep delays with retry-based waits.

Also applies to: 19-19

94-111: LGTM!

The webhook validation is correctly wrapped in assertEventually() with all assertions (including signature validation) performed inside the retry block. This avoids race conditions and ensures the test waits for the index creation webhook before proceeding.

280-297: LGTM!

Consistent with the collection index test above. All webhook assertions are inside the assertEventually block, preventing race conditions.

740-760: LGTM!

The deployment update webhook validation correctly uses assertEventually() with a 10-second timeout, which is appropriate for deployment build operations. All assertions are inside the retry block.

811-831: LGTM!

The execution completion webhook validation uses an appropriate 30-second timeout (longer than other operations) since function executions can legitimately take time. The 1-second polling interval is also reasonable for this longer wait.

tests/e2e/Services/Webhooks/WebhooksBase.php

github-actions · 2025-10-16T09:05:40Z

✨ Benchmark results

Requests per second: 1,217
Requests with 200 status code: 219,073
P99 latency: 0.158557794

⚡ Benchmark Comparison

Metric	This PR	Latest version
RPS	1,217	1,000
200	219,073	179,997
P99	0.158557794	0.196919123

Replace sleep in webhook tests with assertEventually

f0660f3

vermakhushboo requested a review from loks0n October 16, 2025 08:54

coderabbitai bot reviewed Oct 16, 2025

View reviewed changes

tests/e2e/Services/Webhooks/WebhooksBase.php Show resolved Hide resolved

tests/e2e/Services/Webhooks/WebhooksBase.php Show resolved Hide resolved

vermakhushboo marked this pull request as draft October 16, 2025 09:08

Remove race condition

be983eb

vermakhushboo marked this pull request as ready for review October 16, 2025 09:14

loks0n approved these changes Oct 16, 2025

View reviewed changes

loks0n merged commit 5ce9b8e into 1.8.x Oct 16, 2025
41 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace sleep in webhook tests with assertEventually#10656

Replace sleep in webhook tests with assertEventually#10656
loks0n merged 2 commits into1.8.xfrom
feat-replace-sleep-in-webhooks-service

vermakhushboo commented Oct 16, 2025

Uh oh!

coderabbitai bot commented Oct 16, 2025 •

edited

Loading

Walkthrough

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

github-actions bot commented Oct 16, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Oct 16, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

vermakhushboo commented Oct 16, 2025

What does this PR do?

Test Plan

Related PRs and Issues

Checklist

Uh oh!

coderabbitai bot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Possibly related PRs

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

github-actions bot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Scan Results for PR

Docker Image Scan Results

Source Code Scan Results

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✨ Benchmark results

⚡ Benchmark Comparison

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai bot commented Oct 16, 2025 •

edited

Loading

github-actions bot commented Oct 16, 2025 •

edited

Loading

github-actions bot commented Oct 16, 2025 •

edited

Loading