fix(e2e): Use pagination in sentinel #1372
Conversation
|
Warning Rate limit exceeded@elsand has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 11 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request modify the functionality of a test case in Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
|
elsand
force-pushed
the
fix/improve-sentiel-purge
branch
from
81aeddc to
7cb04a8
Compare
![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
🧹 Outside diff range and nitpick comments (2)
tests/k6/common/sentinel.js (2)
11-13: Consider adding JSDoc type documentation for pagination variables.For better code clarity and maintainability, consider documenting the types of these variables.
+/** @type {boolean} */ let hasNextPage = false; +/** @type {string} */ let continuationToken = ""; +/** @type {string[]} */ let dialogIdsToPurge = [];
41-42: Enhance assertion message for better test failure visibility.The current assertion message could be more descriptive to help identify the issue faster.
-expect(dialogIdsToPurge.length, 'unpurged dialogs').to.equal(0); +expect(dialogIdsToPurge.length, `${dialogIdsToPurge.length} dialogs were not properly cleaned up during tests`).to.equal(0);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
tests/k6/common/sentinel.js(1 hunks)
🔇 Additional comments (1)
tests/k6/common/sentinel.js (1)
11-42: Implementation successfully addresses the pagination requirement.
The changes effectively solve the original issue of being limited to the first 100 dialogs. The implementation is solid with proper pagination handling, error checking, and cleanup verification.
elsand
force-pushed
the
fix/improve-sentiel-purge
branch
from
e74675d to
d6f2fa8
Compare
|
🤖 I have created a release *beep* *boop* --- ## [1.28.0](v1.27.1...v1.28.0) (2024-11-05) ### Features * update swagger name generation ([#1350](#1350)) ([94c5544](94c5544)) * **webapi:** Add ExternalReference to dialog search result ([#1384](#1384)) ([431fe16](431fe16)) * **webapi:** Return 410 GONE for notification checks on deleted dialogs ([#1387](#1387)) ([198bebd](198bebd)) ### Bug Fixes * Add system user id to identifying claims ([#1362](#1362)) ([16f160d](16f160d)) * **e2e:** Use pagination in sentinel ([#1372](#1372)) ([a1df0ff](a1df0ff)) * fixed placement of referenced workflow-file ([#1365](#1365)) ([49c1d80](49c1d80)) * workaround for github number error in dispatch workflow ([#1367](#1367)) ([06ee356](06ee356)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Description
This fixes the sentinel to clean all unpurged dialogs, not just the first 100
Related Issue(s)
Verification
Summary by CodeRabbit