-
-
Notifications
You must be signed in to change notification settings - Fork 2
Added assertions for request and response counts. #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Alex Skrypnyk <[email protected]>
WalkthroughThe changes add new assertions to verify the API server's state. Two public methods are introduced in the API server context class to check the number of queued responses and received requests by querying the Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test Scenario
participant C as ApiServerContext
participant S as API Server (/admin/status)
T->>C: Invoke assertQueuedResponsesCount / assertReceivedRequestsCount
C->>S: GET /admin/status
S-->>C: Return headers (X-Queued-Responses, X-Received-Requests)
C-->>T: Log debug message or throw error on mismatch
Possibly related PRs
✨ 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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 1 out of 3 changed files in this pull request and generated no comments.
Files not reviewed (2)
- src/DrevOps/BehatPhpServer/ApiServerContext.php: Language not supported
- tests/behat/features/apiserver.feature: Language not supported
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #38 +/- ##
==========================================
- Coverage 74.87% 73.57% -1.31%
==========================================
Files 3 3
Lines 402 420 +18
==========================================
+ Hits 301 309 +8
- Misses 101 111 +10 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/DrevOps/BehatPhpServer/ApiServerContext.php (1)
369-391
: Implementation of received requests assertion looks good.The method correctly retrieves the API server status and compares the received requests count against the expected value, throwing an appropriate exception if they don't match. The implementation follows the same pattern as other assertion methods.
Consider type casting the count parameters and header values to integers before comparison for more robust numeric comparison:
- if ($received_requests !== $count) { + if ((int)$received_requests !== (int)$count) {This would ensure that string representations like "02" and "2" would be treated as equal.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 383-387: src/DrevOps/BehatPhpServer/ApiServerContext.php#L383-L387
Added lines #L383 - L387 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md
(2 hunks)src/DrevOps/BehatPhpServer/ApiServerContext.php
(1 hunks)tests/behat/features/apiserver.feature
(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/DrevOps/BehatPhpServer/ApiServerContext.php (1)
src/DrevOps/BehatPhpServer/PhpServerContext.php (1)
debug
(625-633)
🪛 GitHub Check: codecov/patch
src/DrevOps/BehatPhpServer/ApiServerContext.php
[warning] 359-363: src/DrevOps/BehatPhpServer/ApiServerContext.php#L359-L363
Added lines #L359 - L363 were not covered by tests
[warning] 383-387: src/DrevOps/BehatPhpServer/ApiServerContext.php#L383-L387
Added lines #L383 - L387 were not covered by tests
🔇 Additional comments (4)
README.md (1)
157-162
: Documentation looks good for the new assertion steps.The added step definitions clearly document the new functionality for asserting the API server's state regarding requests and responses. This will be helpful for users who need to verify that their test scenarios are correctly interacting with the API server.
tests/behat/features/apiserver.feature (2)
148-149
: Good addition of assertions to verify server state.These assertions meaningfully extend the test coverage by validating both that the API server has received exactly the expected number of requests (3) and has emptied its response queue (0), which helps ensure the test scenario has run correctly.
189-190
: Assertions correctly verify API server state after JSON responses.The newly added assertions properly verify the server state after the test operations, confirming that exactly 2 requests were received and no responses remain in the queue.
src/DrevOps/BehatPhpServer/ApiServerContext.php (1)
345-367
:✅ Verification successful
Implementation of queued responses assertion looks good.
The method correctly retrieves the API server status and compares the queued responses count against the expected value, throwing an appropriate exception if they don't match. The implementation is consistent with the rest of the codebase.
🏁 Script executed:
#!/bin/bash # Check test coverage for the new exception code paths # Look for any existing tests that might purposely trigger the exception case rg -A 3 -B 3 "should have \d+ (queued|response)" tests/Length of output: 1207
Queued Responses Assertion Implementation Verified
The implementation in
src/DrevOps/BehatPhpServer/ApiServerContext.php
(lines 345–367) correctly retrieves the API server's status and validates the queued responses count. The test outputs confirm that both the "queued responses" and "responses queued" assertions are being exercised in the feature tests. No further changes are necessary.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 359-363: src/DrevOps/BehatPhpServer/ApiServerContext.php#L359-L363
Added lines #L359 - L363 were not covered by tests
Summary by CodeRabbit