chore(tests): Standardize imports and add page navigavion helper #16914
Conversation
WalkthroughThe changes in this pull request involve modifications to the export statements in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BrowserContext
participant Page
User->>BrowserContext: Create a new context
BrowserContext->>Page: createPageAndNavigate(url)
Page-->>BrowserContext: New page created
Page->>User: Navigate to URL
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
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16914 +/- ##
==========================================
- Coverage 36.46% 36.45% -0.01%
==========================================
Files 6903 6881 -22
Lines 144575 144116 -459
Branches 41282 41131 -151
==========================================
- Hits 52715 52541 -174
+ Misses 91860 91575 -285
Flags with carried forward coverage won't be shown. Click here to find out more. see 41 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
svanaeinars
added
the
deprecated:automerge
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
libs/testing/e2e/src/index.ts (1)
12-13: Well-structured separation of runtime and type exports!This change improves tree-shaking by clearly separating runtime exports (
test,expect) from type-only exports (Page,Locator,BrowserContext). This approach ensures that type information doesn't bloat the production bundle while maintaining strong typing during development.libs/testing/e2e/src/lib/helpers/api-tools.ts (1)
25-39: Consider enhancing the navigation helper with additional options.The implementation is clean and well-documented. Consider these improvements for better robustness:
export const createPageAndNavigate = async ( context: BrowserContext, url: string, + options?: { + timeout?: number; + waitUntil?: 'load'|'domcontentloaded'|'networkidle'; + } ) => { const page = await context.newPage() - await page.goto(url) + try { + await page.goto(url, { + timeout: options?.timeout, + waitUntil: options?.waitUntil ?? 'load' + }) + } catch (error) { + await page.close() + throw error + } return page }This would:
- Add configurable timeout for slow networks
- Allow specifying when navigation is considered complete
- Clean up resources on navigation failure
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
libs/testing/e2e/src/index.ts(1 hunks)libs/testing/e2e/src/lib/helpers/api-tools.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
libs/testing/e2e/src/index.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/testing/e2e/src/lib/helpers/api-tools.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (3)
libs/testing/e2e/src/index.ts (1)
13-13: Verify type-only usage across the codebase
Let's ensure that Page, Locator, and BrowserContext are only used as types throughout the codebase.
✅ Verification successful
Let me gather more specific information about how these types are actually being used in the codebase.
Type exports are used correctly and safely across the codebase
The verification shows that:
- The types
Page,Locator, andBrowserContextare only used as type annotations and parameters in test files - The only runtime usage found is in
pdf.worker.min.mjswhich is an external PDF.js library file, not application code - All imports of these types are properly scoped with
typekeyword in the export statement
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that Page, Locator, and BrowserContext are only used as types
# Check for type usage (should find matches)
echo "Checking for type usage..."
ast-grep --pattern 'type $name = Page;'
ast-grep --pattern 'type $name = Locator;'
ast-grep --pattern 'type $name = BrowserContext;'
ast-grep --pattern 'function $name($param: Page)'
ast-grep --pattern 'function $name($param: Locator)'
ast-grep --pattern 'function $name($param: BrowserContext)'
# Check for potential runtime usage (should not find matches)
echo "Checking for potential runtime usage..."
rg -l 'new (Page|Locator|BrowserContext)\('
rg -l 'instanceof (Page|Locator|BrowserContext)'
Length of output: 640
Script:
#!/bin/bash
# Search for all imports and type references of Page, Locator, and BrowserContext
echo "Checking imports and type references..."
rg -A 2 "import.*\{.*(Page|Locator|BrowserContext).*\}" --type ts
echo "\nChecking type annotations..."
rg ":\s*(Page|Locator|BrowserContext)[\s,\)]" --type ts
# Check the specific file that showed runtime usage
echo "\nChecking the pdf.worker.min.mjs file..."
head -n 20 apps/web/public/assets/pdf.worker.min.mjs
Length of output: 118564
libs/testing/e2e/src/lib/helpers/api-tools.ts (2)
Line range hint 1-24: LGTM! Clean imports and well-documented existing function.
The code demonstrates good TypeScript practices with proper type imports and comprehensive JSDoc documentation.
Line range hint 1-39: Excellent compliance with library coding guidelines.
The implementation successfully meets all requirements for library code:
- ✓ Functions are reusable across different NextJS e2e test suites
- ✓ Proper TypeScript types and documentation
- ✓ Tree-shakeable exports with no side effects
Datadog ReportAll test runs ✅ 3 Total Test Services: 0 Failed, 3 Passed Test Services
|
kodiakhq
bot
deleted the
chore/standardize-imports-and-add-page-navigation-e2e
branch
Summary by CodeRabbit
createPageAndNavigate, for creating and navigating to a page within a specified browser context.testandexpect, while convertingPage,Locator, andBrowserContextto type-only exports.