Add tests for empty output directory #9566
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces three new test methods in the Changes
Possibly related PRs
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
✨ Benchmark results
⚡ Benchmark Comparison
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/e2e/Services/Sites/SitesCustomServerTest.php (2)
2264-2300: Good test for the empty output directory case.The test correctly verifies that deployments fail when the output directory exists but is empty. You've set up the test to create the empty directory with
mkdir randomand then start the build process.To make this test more robust, consider adding an assertion to verify the specific error message in the build logs, confirming that the failure is due to an empty output directory rather than some other unrelated error.
$this->assertEventually(function () use ($siteId, $deploymentId) { $deployment = $this->getDeployment($siteId, $deploymentId); $this->assertEquals('failed', $deployment['body']['status'], 'Deployment status is failed, deployment: ' . json_encode($deployment['body'], JSON_PRETTY_PRINT)); + $this->assertStringContainsString('Build output directory is empty', $deployment['body']['buildLogs']); }, 100000, 500);
2302-2338: Good test for the non-existent output directory case.The test correctly verifies that deployments fail when the specified output directory doesn't exist. This test complements the previous one by covering another empty output directory scenario.
Similar to the previous test, consider adding an assertion to verify the specific error message in the build logs to ensure the failure is occurring for the expected reason.
$this->assertEventually(function () use ($siteId, $deploymentId) { $deployment = $this->getDeployment($siteId, $deploymentId); $this->assertEquals('failed', $deployment['body']['status'], 'Deployment status is failed, deployment: ' . json_encode($deployment['body'], JSON_PRETTY_PRINT)); + $this->assertStringContainsString('Build output directory is not set or does not exist', $deployment['body']['buildLogs']); }, 100000, 500);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/e2e/Services/Sites/SitesCustomServerTest.php(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
tests/e2e/Services/Sites/SitesCustomServerTest.php (1)
tests/e2e/Services/Sites/SitesBase.php (6)
setupSite(20-33)getSite(161-169)setupSiteDomain(329-347)createDeployment(237-245)getDeployment(171-179)cleanupSite(69-78)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Setup & Build Appwrite Image
What does this PR do?
Need to release OPR version for this to pass
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
Checklist
Summary by CodeRabbit