feat(tests): Add functions and documentation to testing/e2e #16694
Conversation
WalkthroughThe pull request introduces several changes focused on enhancing end-to-end (E2E) testing capabilities. Key modifications include updating the Changes
Possibly related PRs
Suggested labels
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
|
svanaeinars
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 43
🧹 Outside diff range and nitpick comments (12)
libs/testing/e2e/src/lib/utils/pageHelpers.ts (1)
1-2: Add TypeScript documentation for better developer experience.Since this is a reusable testing library, adding JSDoc comments would improve the developer experience.
Add documentation:
+/** + * Returns a CSS selector for an input element with the specified name attribute. + * @param name - The value of the name attribute to match + * @returns A CSS selector string + * @example + * const selector = getInputByName('email'); + * // Returns: input[name="email"] + */ export const getInputByName = (name: string) => `input[name="${name}"]` +/** + * Returns a CSS selector for a textarea element with the specified name attribute. + * @param name - The value of the name attribute to match + * @returns A CSS selector string + * @example + * const selector = getTextareaByName('description'); + * // Returns: textarea[name="description"] + */ export const getTextareaByName = (name: string) => `textarea[name="${name}"]`libs/testing/e2e/src/lib/support/i18n.ts (1)
15-15: Add JSDoc documentation for better code maintainability.The exported function would benefit from proper documentation explaining its purpose and usage.
Here's a suggested improvement:
+/** + * Formats a message descriptor using the configured intl instance + * @param {MessageDescriptor} l - The message descriptor to format + * @returns {string} The formatted message + */ export const label = (l: MessageDescriptor) => intl.formatMessage(l)libs/testing/e2e/src/index.ts (1)
13-13: Consider moving third-party export to top.It's a common convention to place third-party imports/exports before local ones. This helps with:
- Distinguishing external vs internal dependencies
- Maintaining consistent code organization
+export * from '@playwright/test' + export * from './lib/support/api-tools' export { createApplication } from './lib/support/application' export * from './lib/support/disablers' // ... rest of local exports -export * from '@playwright/test'libs/testing/e2e/src/lib/utils/playwright-config.ts (1)
3-8: Add JSDoc documentation to improve interface clarity.While the interface is well-structured, adding JSDoc comments would improve documentation and provide better IDE support.
+/** + * Parameters for creating a global Playwright configuration + */ interface GlobalConfigParams { + /** Base URL for the web server running the application under test */ webServerUrl: string + /** Optional port number for the web server */ port?: number + /** Command to start the web server */ command: string + /** Optional working directory for the web server, defaults to '../../../' */ cwd?: string }libs/testing/e2e/src/lib/support/application.ts (1)
3-13: Enhance JSDoc documentationWhile the documentation is comprehensive, consider adding:
@throwstag to document potential error scenarios (e.g., network failures, timeout errors)- More specific return type documentation (e.g.,
Promise<number>representing the count of existing applications)/** Creates a new application and returns the number of applications before creation. @async @function @param {Page} page - Playwright Page object representing the current page. @returns {Promise<number>} - The number of applications before the new application is created. + @throws {Error} - When the GraphQL request fails or times out + @throws {Error} - When the create application button is not found This function waits for the applications to load on the overview page and counts the number of applications. If there is an existing application, the overview page will not redirect to a new application. In this case, the function clicks the 'create-new-application' button to create a new application. */.gitignore (1)
102-105: Consider adding trailing slashes for consistency.While the ignore patterns work correctly, consider adding trailing slashes to explicitly indicate these are directories, matching the style used elsewhere in the file (e.g.,
node_modules/).# E2E outputs -test-results/ -playwright-report/ -tmp-sessions/ +test-results// +playwright-report// +tmp-sessions//libs/testing/e2e/README.md (2)
1-11: Enhance documentation with TypeScript and reusability details.Consider adding information about:
- TypeScript usage in the helper functions and configurations
- How these utilities can be reused across different NextJS applications
Example addition after line 10:
- **Global Playwright Configuration:** The `createGlobalConfig` function provides a shared Playwright configuration used across multiple applications. It standardizes the testing environment. + **Global Playwright Configuration:** The `createGlobalConfig` function provides a shared Playwright configuration used across multiple applications. It standardizes the testing environment. + + ## TypeScript Support + + All helper functions and configurations are written in TypeScript, providing type safety and better IDE support. The library exports TypeScript interfaces and types that can be used in your test files. + + ## Reusability + + The utility functions are designed to be reusable across different NextJS applications within the project. They abstract common testing patterns and provide a consistent testing experience regardless of the specific application being tested.
46-48: Enhance CLI command documentation.The command example could benefit from:
- A more specific example path
- Information about required environment variables
- Common port numbers used in the project
Example enhancement:
```bash -yarn mockoon-cli start --data ./apps/<my-app>/e2e/mocks/<my-app-mock>.json --port <port> +yarn mockoon-cli start --data ./apps/web/e2e/mocks/api-mock.json --port 3000Note: Replace the port number with one that doesn't conflict with your application's default port. Common ports used in the project are 3000, 3001, and 3002.
</blockquote></details> <details> <summary>libs/testing/e2e/src/lib/support/urls.ts (1)</summary><blockquote> `19-25`: **Document the purpose of hardcoded national IDs** The login URLs contain hardcoded national IDs without explanation. Add documentation explaining what these IDs represent (e.g., test accounts, mock data) and their specific roles. ```diff +// National IDs for test accounts: +// 9999999999 - Regular user +// 0000000000 - Judge +// 0000000001 - Court of Appeals Judge +// 0909090909 - Defender export const JUDICIAL_SYSTEM_HOME_URL = '/api/auth/login?nationalId=9999999999' export const JUDICIAL_SYSTEM_JUDGE_HOME_URL = '/api/auth/login?nationalId=0000000000'libs/testing/e2e/src/lib/support/login.ts (1)
71-74: Address the TODO: Update selector when the new login page is deployed.There's a TODO comment indicating that an accessible selector should be used once the new login page is available. Updating the selector will improve accessibility and maintainability.
Do you want assistance in updating the selector now, or should we open a GitHub issue to track this task?
libs/testing/e2e/src/lib/support/email-account.ts (1)
83-92: Add explicit TypeScript type annotations for 'emailConfig'To enhance type safety and readability, consider adding explicit TypeScript type annotations for
emailConfig.For example:
+ import { ImapSimpleOptions } from 'imap-simple' const emailConfig: ImapSimpleOptions = { imap: { user: testAccount.user, password: testAccount.pass, host: 'imap.ethereal.email', port: 993, tls: true, authTimeout: 10000, }, }libs/testing/e2e/src/lib/support/session.ts (1)
153-153: Ensure consistent URL matching in session validationThe assertion
await expect(sessionValidation?.url()).toMatch(homeUrl)assumes that the navigated URL will exactly matchhomeUrl. IfhomeUrlis not a full URL or if there are redirects or added query parameters, this assertion might fail. Consider usingtoContainor normalizing URLs before comparison.Apply this diff to adjust the assertion:
- await expect(sessionValidation?.url()).toMatch(homeUrl) + await expect(sessionValidation?.url()).toContain(homeUrl)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (19)
.gitignore(1 hunks)libs/testing/e2e/README.md(1 hunks)libs/testing/e2e/src/index.ts(1 hunks)libs/testing/e2e/src/lib/libs/testing/e2e.spec.ts(0 hunks)libs/testing/e2e/src/lib/libs/testing/e2e.ts(0 hunks)libs/testing/e2e/src/lib/support/api-tools.ts(1 hunks)libs/testing/e2e/src/lib/support/application.ts(1 hunks)libs/testing/e2e/src/lib/support/disablers.ts(1 hunks)libs/testing/e2e/src/lib/support/email-account.ts(1 hunks)libs/testing/e2e/src/lib/support/i18n.ts(1 hunks)libs/testing/e2e/src/lib/support/locator-helpers.ts(1 hunks)libs/testing/e2e/src/lib/support/login.ts(1 hunks)libs/testing/e2e/src/lib/support/session.ts(1 hunks)libs/testing/e2e/src/lib/support/urls.ts(1 hunks)libs/testing/e2e/src/lib/support/utils.ts(1 hunks)libs/testing/e2e/src/lib/utils/addons.ts(1 hunks)libs/testing/e2e/src/lib/utils/pageHelpers.ts(1 hunks)libs/testing/e2e/src/lib/utils/playwright-config.ts(1 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (2)
- libs/testing/e2e/src/lib/libs/testing/e2e.spec.ts
- libs/testing/e2e/src/lib/libs/testing/e2e.ts
🧰 Additional context used
📓 Path-based instructions (15)
libs/testing/e2e/README.md (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/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/support/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."
libs/testing/e2e/src/lib/support/application.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/support/disablers.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/support/email-account.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/support/i18n.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/support/locator-helpers.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/support/login.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/support/session.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/support/urls.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/support/utils.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/utils/addons.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/utils/pageHelpers.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/utils/playwright-config.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."
🪛 LanguageTool
libs/testing/e2e/README.md
[grammar] ~20-~20: The verb ‘mocks’ is singular. Did you mean: “this mocks” or “these mock”?
Context: ...ommand-line interface (CLI) for running these mocks in various environments, such as pipeli...
(SINGULAR_VERB_AFTER_THESE_OR_THOSE)
🪛 Biome
libs/testing/e2e/src/lib/support/email-account.ts
[error] 35-35: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (19)
libs/testing/e2e/src/index.ts (2)
2-2: LGTM! Good use of named export.
The explicit named export of createApplication follows best practices for:
- Clear dependency tracking
- Effective tree-shaking
- Self-documenting API
1-13: Verify potential circular dependencies.
With multiple interdependent testing utilities, it's important to ensure there are no circular dependencies that could cause issues.
✅ Verification successful
Let me try another approach to check for circular dependencies with corrected regex patterns.
Let me try one final check to look for imports with a more inclusive pattern that might catch any potential circular dependencies.
No circular dependencies detected in the testing utilities
Based on the analysis of import statements across all support and utils files, the codebase maintains a clean dependency structure:
- Support modules have clear, one-way dependencies (e.g., login depends on urls and utils)
- Utils modules are mostly self-contained with external dependencies
- No circular import chains were found between modules
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential circular dependencies in the testing library
# Test 1: Look for files importing from index.ts
echo "Files importing from index.ts:"
rg -l "from '\.\.\/index'" libs/testing/e2e/src/
# Test 2: Check for cross-imports between support modules
echo -e "\nCross-imports between support modules:"
for file in $(fd -t f . libs/testing/e2e/src/lib/support/); do
echo "=== Imports in $file ==="
rg "from '.*support/" "$file" || true
done
Length of output: 3138
Script:
#!/bin/bash
# Check for circular dependencies with corrected patterns
# Look for imports between files
echo "=== Checking cross-imports in support modules ==="
for file in $(fd -t f . libs/testing/e2e/src/lib/support/); do
filename=$(basename "$file")
echo -e "\nFile: $filename"
rg "from ['\"]\.\.?/support/" "$file" || true
done
echo -e "\n=== Checking imports from index.ts ==="
rg "from ['\"]\.\.?/index" libs/testing/e2e/src/lib/
# Check utils folder imports as well
echo -e "\n=== Checking utils folder imports ==="
for file in $(fd -t f . libs/testing/e2e/src/lib/utils/); do
filename=$(basename "$file")
echo -e "\nFile: $filename"
rg "from ['\"]\.\.?/(support|utils)/" "$file" || true
done
Length of output: 4121
Script:
#!/bin/bash
# Final check for circular dependencies with broader patterns
echo "=== Checking all imports in support files ==="
for file in $(fd -t f . libs/testing/e2e/src/lib/support/); do
echo -e "\nFile: $(basename "$file")"
rg "^import .* from ['\"]" "$file" || true
done
echo -e "\n=== Checking all imports in utils files ==="
for file in $(fd -t f . libs/testing/e2e/src/lib/utils/); do
echo -e "\nFile: $(basename "$file")"
rg "^import .* from ['\"]" "$file" || true
done
Length of output: 4790
libs/testing/e2e/src/lib/support/locator-helpers.ts (1)
1-24: LGTM! Code follows the library guidelines.
The implementation:
- Uses TypeScript effectively with proper type definitions
- Provides reusable helper functions
- Follows proper export patterns enabling tree-shaking
libs/testing/e2e/src/lib/support/utils.ts (1)
5-5: LGTM! Well-implemented utility function.
The sleep function is properly typed and follows Promise-based implementation best practices.
libs/testing/e2e/src/lib/utils/playwright-config.ts (2)
16-43: LGTM! The configuration structure follows Playwright best practices.
The configuration includes essential settings for both development and CI environments, with proper handling of:
- Parallel execution
- Retries and workers
- Tracing
- Web server management
35-38: Clarify web server URL and port configuration logic.
The current logic for handling URL and port is not immediately clear. Consider making this more explicit or adding validation.
Let's check if this configuration pattern is used consistently across the codebase:
✅ Verification successful
The URL and port configuration logic is clear and correct
The configuration follows a standard pattern where:
- When
portis provided, it's used for starting the web server and the URL is automatically derived - When
portis not provided, a specificwebServerUrlmust be used instead
This is a common and valid configuration pattern for Playwright, where you either:
- Specify a port and let Playwright handle the URL construction
- Provide a complete URL when the server is already running elsewhere
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other Playwright configurations to verify URL/port handling patterns
rg -l "webServer.*port.*url" --type ts
Length of output: 40
Script:
#!/bin/bash
# Let's try a broader search for Playwright configuration patterns
rg -A 5 "webServer\s*:\s*{" --type ts
# Also search for specific port and url configurations
rg -A 3 "port\s*:\s*" --type ts
# Look for the actual file content
cat libs/testing/e2e/src/lib/utils/playwright-config.ts
Length of output: 59539
libs/testing/e2e/src/lib/utils/addons.ts (1)
1-1: LGTM: Clean and well-structured import statement
The import statement follows TypeScript best practices and includes only the necessary utilities.
libs/testing/e2e/src/lib/support/application.ts (2)
1-1: LGTM: Clean import with proper type usage
The import statement correctly imports the Page type from Playwright, ensuring proper type safety.
16-18: Verify GraphQL operation in schema
Let's ensure the GraphQL operation is properly typed and documented in the schema.
✅ Verification successful
The previous search had issues with file type recognition. Let's try a broader search to find any references to this operation and its definition.
GraphQL operation is properly typed and used across the application
The ApplicationApplications operation is well-defined and properly used throughout the codebase:
- It's defined as a GraphQL query in multiple locations with proper typing (
ApplicationApplicationsInput) - Used consistently in both frontend and backend:
- Frontend: Used in service portal, native app, and e2e tests
- Backend: Implemented in the application domain with proper resolvers and service layer
- The operation in the e2e test file matches the schema definition and its usage across the application
The GraphQL operation being waited for in the e2e test is correctly implemented and typed in the schema.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for ApplicationApplications query definition in GraphQL schema files
echo "Searching for ApplicationApplications query definition..."
fd -e graphql -e gql | xargs rg -l "type Query" | while read -r file; do
echo "Checking file: $file"
rg -A 5 "applicationApplications" "$file"
done
# Search for any tests using this query
echo "Searching for existing tests..."
rg -l "ApplicationApplications" --type=typescript --type=javascript
Length of output: 1067
Script:
#!/bin/bash
# Let's try a different approach to search for the GraphQL operation
echo "Searching for ApplicationApplications in all files..."
rg -i "ApplicationApplications" -A 5 -B 5
# Also search for any GraphQL query definitions
echo "Searching for GraphQL query definitions..."
rg "query.*ApplicationApplications" -A 5
Length of output: 53577
.gitignore (1)
101-105: LGTM! Appropriate additions for E2E testing outputs.
The new entries correctly exclude Playwright's test results, reports, and temporary session data from version control, which is the right approach for generated test artifacts.
package.json (1)
356-356: Verify @playwright/test peer dependency.
The @nx/playwright package typically requires @playwright/test as a peer dependency. While it's present in the dependencies, ensure it's at a compatible version.
#!/bin/bash
# Check if @playwright/test version is compatible with @nx/playwright
rg -A 2 "@playwright/test.*\"1\.48\"" package.jsonlibs/testing/e2e/src/lib/support/login.ts (5)
5-8: Type definition for 'CognitoCreds' utilizes TypeScript effectively.
The CognitoCreds type is properly defined with username and password fields, enhancing type safety and clarity in credential management.
10-18: getCognitoCredentials function correctly retrieves and validates credentials.
The function appropriately accesses environment variables and throws an error if credentials are missing, ensuring that authentication cannot proceed without valid inputs.
31-46: cognitoLogin function correctly handles the Cognito authentication flow.
The function successfully automates the login process, including form interactions and navigation handling. The use of optional credentials parameter enhances reusability.
48-93: idsLogin function effectively manages login through the identity service.
The function accurately handles phone number input, delegation selection, and waits for appropriate page navigation. The comments provide clarity on the handling of both new and current login pages.
95-109: switchUser function accurately switches between user profiles.
The function correctly interacts with the user menu and handles optional user selection. Waiting for page navigation ensures the user interface updates as expected.
libs/testing/e2e/src/lib/support/disablers.ts (1)
15-15: 🛠️ Refactor suggestion
Enhance reusability by ensuring flexible parameter types.
By updating the type of deepMockKey to accept both Matchable and Matchable[], you improve the function's flexibility and reusability across different scenarios.
As updated previously:
deepMockKey?: Matchable | Matchable[]Likely invalid or redundant comment.
libs/testing/e2e/src/lib/support/session.ts (2)
19-38: Function ensureCognitoSessionIfNeeded is well-implemented
The function correctly identifies when Cognito authentication is required and performs the login process appropriately. The use of page.request.get to validate the session and handling redirects is effective.
159-176: Function judicialSystemSession is correctly implemented
The function properly sets up a new browser context and ensures the Cognito session for the judicial system. This is a good abstraction for tests specific to the judicial system context.
| export const getInputByName = (name: string) => `input[name="${name}"]` | ||
| export const getTextareaByName = (name: string) => `textarea[name="${name}"]` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider refactoring to reduce code duplication.
Both functions share similar logic for generating selectors. This could be abstracted into a reusable helper.
Consider this refactor:
+/**
+ * Returns a CSS selector for a form element with the specified element type and name attribute.
+ * @param elementType - The HTML element type (e.g., 'input', 'textarea')
+ * @param name - The value of the name attribute to match
+ * @returns A CSS selector string
+ */
+const getElementByName = (elementType: string, name: string) =>
+ `${elementType}[name="${name}"]`
+
+/**
+ * Returns a CSS selector for an input element with the specified name attribute.
+ */
+export const getInputByName = (name: string) => getElementByName('input', name)
+
+/**
+ * Returns a CSS selector for a textarea element with the specified name attribute.
+ */
+export const getTextareaByName = (name: string) => getElementByName('textarea', name)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getInputByName = (name: string) => `input[name="${name}"]` | |
| export const getTextareaByName = (name: string) => `textarea[name="${name}"]` | |
| /** | |
| * Returns a CSS selector for a form element with the specified element type and name attribute. | |
| * @param elementType - The HTML element type (e.g., 'input', 'textarea') | |
| * @param name - The value of the name attribute to match | |
| * @returns A CSS selector string | |
| */ | |
| const getElementByName = (elementType: string, name: string) => | |
| `${elementType}[name="${name}"]` | |
| /** | |
| * Returns a CSS selector for an input element with the specified name attribute. | |
| */ | |
| export const getInputByName = (name: string) => getElementByName('input', name) | |
| /** | |
| * Returns a CSS selector for a textarea element with the specified name attribute. | |
| */ | |
| export const getTextareaByName = (name: string) => getElementByName('textarea', name) |
Add input validation and escape special characters.
The functions directly interpolate the input into CSS selectors without validation, which could lead to CSS injection if the name contains special characters.
Consider adding input validation and escaping:
+const escapeCSSString = (str: string): string => {
+ return CSS.escape(str);
+}
+
-export const getInputByName = (name: string) => `input[name="${name}"]`
-export const getTextareaByName = (name: string) => `textarea[name="${name}"]`
+export const getInputByName = (name: string) => `input[name="${escapeCSSString(name)}"]`
+export const getTextareaByName = (name: string) => `textarea[name="${escapeCSSString(name)}"]`Committable suggestion skipped: line range outside the PR's diff.
| @@ -1 +1,13 @@ | |||
| export * from './lib/libs/testing/e2e' | |||
| export * from './lib/support/api-tools' | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using named exports for better tree-shaking.
While using export * is convenient, it can make tree-shaking less effective. Consider explicitly listing the exports from each module to:
- Make dependencies more visible
- Enable better dead code elimination
- Provide better documentation of available functionality
Example refactor for api-tools:
-export * from './lib/support/api-tools'
+export {
+ graphqlSpy,
+ mockApi,
+ verifyRequestCompletion
+} from './lib/support/api-tools'Also applies to: 3-10
| async getLastEmail(retries: number): Promise<null | { | ||
| subject: string | undefined | ||
| text: string | undefined | ||
| html: string | false | ||
| }> { | ||
| // makes debugging very simple | ||
| debug('getting the last email') | ||
| debug(JSON.stringify(emailConfig)) | ||
|
|
||
| debug('connecting to mail server...') | ||
| const connection = await connect(emailConfig) | ||
| debug('connected') | ||
| try { | ||
| // grab up to 50 emails from the inbox | ||
| debug('Opening inbox...') | ||
| await connection.openBox('INBOX') | ||
| debug('Opened inbox.') | ||
| const searchCriteria = ['UNSEEN'] | ||
| const fetchOptions = { | ||
| bodies: [''], | ||
| markSeen: true, | ||
| } | ||
| debug('Starting search for new messages...') | ||
| const messages = await connection.search(searchCriteria, fetchOptions) | ||
| debug('Search finished') | ||
|
|
||
| if (!messages.length) { | ||
| debug('cannot find any emails') | ||
| if (retries <= 0) { | ||
| return null | ||
| } else { | ||
| await new Promise((r) => setTimeout(r, 5000)) | ||
| return userEmail.getLastEmail(retries - 1) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor 'getLastEmail' to avoid recursion and prevent stack overflows
The getLastEmail function uses recursion for retry logic, which can lead to stack overflows if the number of retries is large. Consider refactoring to use an iterative approach.
Here's an example refactor:
async getLastEmail(retries: number): Promise<null | {
subject: string | undefined
text: string | undefined
html: string | false
}> {
- // Existing recursive implementation
+ let attempts = retries
+ while (attempts >= 0) {
+ debug('connecting to mail server...')
+ const connection = await connect(emailConfig)
+ debug('connected')
+ try {
+ // ... (rest of your logic)
+ if (!messages.length) {
+ debug('cannot find any emails')
+ if (attempts === 0) {
+ return null
+ } else {
+ await new Promise((r) => setTimeout(r, 5000))
+ attempts--
+ continue
+ }
+ } else {
+ // Return the email
+ return {
+ subject: mail.subject,
+ text: mail.text,
+ html: mail.html,
+ }
+ }
+ } finally {
+ connection.end()
+ }
+ }
}Committable suggestion skipped: line range outside the PR's diff.
Datadog ReportAll test runs ✅ 101 Total Test Services: 0 Failed, 98 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (3) |
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
.gitignorefile to exclude temporary E2E testing outputs, streamlining version control.Chores
@nx/playwrightto enhance testing capabilities.