-
Notifications
You must be signed in to change notification settings - Fork 60
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
feat(tests): New @island/testing/e2e library #16287
Conversation
WalkthroughThis pull request introduces several new configuration files and a README for 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
|
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
🧹 Outside diff range and nitpick comments (6)
libs/testing/e2e/src/lib/libs/testing/e2e.ts (1)
1-3
: Good implementation, but consider enhancing documentation and purpose.The function is well-implemented with proper TypeScript usage. The named export is favorable for tree-shaking. However, the purpose of this function is not immediately clear.
Consider adding a JSDoc comment to explain the function's purpose and usage. For example:
/** * Returns the identifier for the e2e testing library. * This can be used for logging, configuration, or other identification purposes. * @returns {string} The identifier string for the e2e testing library. */ export function libsTestingE2e(): string { return 'libs/testing/e2e' }If this function is intended to provide a constant identifier, consider using a named constant instead:
export const E2E_LIBRARY_IDENTIFIER = 'libs/testing/e2e'This approach would be more efficient and clearer in its intent.
libs/testing/e2e/src/lib/libs/testing/e2e.spec.ts (1)
1-7
: Test suite structure looks good, but consider expanding test coverage.The test suite is correctly structured using Jest's
describe
andit
functions, and the import statement is properly defined. However, there's room for improvement:
- Consider adding more test cases to cover different scenarios and edge cases of the
libsTestingE2e
function.- Add type annotations to improve TypeScript usage, especially for function parameters and return types.
Here's an example of how you could expand the test suite:
import { libsTestingE2e } from './libs/testing/e2e'; describe('libsTestingE2e', () => { it('should return the correct string', () => { expect(libsTestingE2e()).toEqual('libs/testing/e2e'); }); it('should always return a string', () => { const result: string = libsTestingE2e(); expect(typeof result).toBe('string'); }); // Add more test cases as needed });libs/testing/e2e/tsconfig.spec.json (1)
8-13
: LGTM: Comprehensive inclusion of test files.The
include
section is well-configured:
- It includes the Jest configuration file, which is necessary for the test setup.
- The patterns for test files are comprehensive, covering both
.test.ts
and.spec.ts
conventions.- Including
.d.ts
files ensures that type declaration files are properly processed.This configuration supports TypeScript usage for testing, aligning with the coding guidelines for the
libs
directory.For improved clarity, consider combining the test file patterns:
"include": [ "jest.config.ts", - "src/**/*.test.ts", - "src/**/*.spec.ts", - "src/**/*.d.ts" + "src/**/*.{test,spec,d}.ts" ]This change doesn't affect functionality but makes the configuration more concise.
libs/testing/e2e/project.json (3)
7-13
: LGTM: Lint configuration is properly set up.The lint target is well-configured using ESLint for TypeScript files. However, consider adding
.tsx
files to the lint patterns for broader coverage:- "lintFilePatterns": ["libs/testing/e2e/**/*.ts"] + "lintFilePatterns": ["libs/testing/e2e/**/*.{ts,tsx}"]This change ensures that any potential JSX/TSX files in the e2e tests are also linted.
14-27
: Test configuration looks good, but consider enforcing test presence.The test target is well-configured using Jest, with appropriate settings for CI and code coverage. However, for an e2e testing library, it might be beneficial to ensure that tests are always present:
- "passWithNoTests": true + "passWithNoTests": falseThis change would cause the test run to fail if no tests are found, encouraging developers to always include tests in this critical library.
29-29
: Consider adding relevant tags to the project.The
tags
array is currently empty. Adding relevant tags can improve project organization and discoverability within your monorepo. Consider adding tags such as:- "tags": [] + "tags": ["scope:e2e", "type:testing", "target:browser"]These tags would clearly indicate the purpose and scope of this library, making it easier to manage and locate within your project structure.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (11)
- libs/testing/e2e/.eslintrc.json (1 hunks)
- libs/testing/e2e/README.md (1 hunks)
- libs/testing/e2e/jest.config.ts (1 hunks)
- libs/testing/e2e/project.json (1 hunks)
- libs/testing/e2e/src/index.ts (1 hunks)
- libs/testing/e2e/src/lib/libs/testing/e2e.spec.ts (1 hunks)
- libs/testing/e2e/src/lib/libs/testing/e2e.ts (1 hunks)
- libs/testing/e2e/tsconfig.json (1 hunks)
- libs/testing/e2e/tsconfig.lib.json (1 hunks)
- libs/testing/e2e/tsconfig.spec.json (1 hunks)
- tsconfig.base.json (1 hunks)
✅ Files skipped from review due to trivial changes (6)
- libs/testing/e2e/.eslintrc.json
- libs/testing/e2e/README.md
- libs/testing/e2e/jest.config.ts
- libs/testing/e2e/src/index.ts
- libs/testing/e2e/tsconfig.json
- libs/testing/e2e/tsconfig.lib.json
🧰 Additional context used
📓 Path-based instructions (4)
libs/testing/e2e/project.json (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/libs/testing/e2e.spec.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/libs/testing/e2e.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/tsconfig.spec.json (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 (5)
libs/testing/e2e/tsconfig.spec.json (3)
2-2
: LGTM: Proper extension from base configuration.Extending from a base
tsconfig.json
is a good practice. It promotes consistency and reduces configuration duplication across the project.
3-7
: LGTM: Compiler options are well-configured for testing.The compiler options are appropriately set for a testing environment:
- The
outDir
setting organizes compiled files efficiently.- Using "commonjs" as the module system is suitable for Node.js environments, which is typical for testing.
- Including types for Jest and Node.js ensures proper type checking in test files.
These settings align with the coding guidelines for effective bundling practices in the
libs
directory.
1-14
: Overall assessment: Well-configured TypeScript setup for e2e testing.This
tsconfig.spec.json
file is well-structured and follows good practices for TypeScript configuration in a testing context. It aligns with the coding guidelines for files in thelibs
directory:
- It supports reusability across different NextJS apps by providing a consistent testing setup.
- It properly uses TypeScript for testing purposes, including necessary type definitions.
- The configuration supports effective tree-shaking and bundling practices through appropriate compiler options.
The file successfully sets up a TypeScript environment tailored for end-to-end testing, which will contribute to maintaining code quality and type safety in the testing process.
libs/testing/e2e/project.json (1)
1-5
: LGTM: Project metadata is well-defined.The project metadata is correctly configured for an e2e testing library in an Nx workspace. The project name, schema reference, sourceRoot, and projectType are all appropriately set.
tsconfig.base.json (1)
1115-1115
: LGTM: New path mapping for e2e testing library added.The addition of the path mapping for "@island.is/testing/e2e" is consistent with the PR objectives and enhances the module resolution capabilities for end-to-end testing components.
Let's verify if there are any other files that might need updates due to this new path mapping:
✅ Verification successful
Action Required: Correct the search command to properly target TypeScript and JavaScript files.
The previous search failed because
typescript
is not a recognized file type inrg
. Please run the following script to search within.ts
and.tsx
files:
Verified: No additional files require updates due to the new path mapping for "@island.is/testing/e2e".
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for files that might need updates due to the new path mapping rg --type typescript --type javascript -i '@island.is/testing/e2e'Length of output: 101
Script:
#!/bin/bash # Search for files that might need updates due to the new path mapping rg -g '*.ts' -g '*.tsx' -i '@island.is/testing/e2e'Length of output: 51
Datadog ReportAll test runs ✅ 70 Total Test Services: 0 Failed, 68 Passed Test ServicesThis report shows up to 10 services
|
* Add @swc-node/register and @swc/core * Add testing/e2e library * update project.json for testing/e2e * fix import for libTestingE2e --------- Co-authored-by: Kristofer <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
…-pages (#16234) * Service portal removal. Add portals my pages * minor fixes * Fix * path fix * fix(portals-admin): locklist (#16279) * fix(portals-admin): locklist * tweak * msg id fix * tweak --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(service-portal): feature flag resolver for documents (#16285) * fix: def info and alert * feat: add feature flag to resolver * fix: move ff call to seperate function --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(vehicles-bulk-mileage): Fixes after testing review (#16295) * fix: testing fixes v1 * fix: testing comments v2 * fix: better message * fix: function name * fix: duplicate loading --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(tests): New @island/testing/e2e library (#16287) * Add @swc-node/register and @swc/core * Add testing/e2e library * update project.json for testing/e2e * fix import for libTestingE2e --------- Co-authored-by: Kristofer <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(parental-leave): ApplicationRights (#15901) * feat(parental-leave): ApplicationRights Added applicationRights to parental-leave when sending application. Since we are using a new way of calculating periods * Fix days used by period calculation * Tests for new periods * rename function with proper camelCase * Refactor: Made duplicate code into a function * Make ApplicationRights nullable * refactor: function instead of duplicate code * remove console.log * error handling for period data * clientConfig nullable fix * Fixes for calculation of months. And using clamp to get correct value of daysLeft * Multiply amount of months by 30 for period calculation with month durations * Fix old calculation of endDate with months --------- Co-authored-by: hfhelgason <[email protected]> Co-authored-by: veronikasif <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(passport-application): Updated readme (#16296) * updated readme * updated readme * chore: nx format:write update dirty files --------- Co-authored-by: andes-it <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(regulations-admin): date format signature, remove self affect, disclaimer text (#16288) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(regulations-admin): No diff no addition in appendix (#16293) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(web): Global alert banner - Handle null case (#16298) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(web): Change custom syslumenn pages config for header (#16299) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(j-s): Digital mailbox API (#16301) * feat(j-s): Block create subpoena on staging and dev * Update subpoena.service.ts * fix(j-s): Fix mailbox API * remove changes not meant for this branch * Update subpoena.service.ts * fix(j-s): reverting changes from other branch * Update subpoena.response.ts * Update subpoena.response.ts * Update subpoena.response.ts * Update subpoena.response.ts --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(signature-collection): Fix list reviewed toggle (#16300) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * chore(scripts): Stricter shell script checking (#16242) * Set style level for shellcheck * Linting & formatting scripts * Remove _podman.sh script * Format all scripts * Add reviewdog/action-shfmt step * Configure shfmt * Merge from main * Linting * Move shfmt to before lint * Remove reviewdog * Allow external sources in shellcheck * Use Reviewdog for shellcheck * Set version for Reviewdog --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * chore(new-primary-school): Update messages namespace (#16302) Co-authored-by: veronikasif <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(driving-license): check if 65+ renewal is possible (#16292) * check if 65 renewal is possible * remove console log * cleanup * coderabbit tweaks * coderabbit changes * quick fix * add type? --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(service-portal): default defender and has chosen fields for subpoena (#16306) * fix: def info and alert * feat: add feature flag to resolver * fix: move ff call to seperate function * feat: add default choices ans has chosen + loading states * fix: use type * fix: undefined type issue * fix: simplify check * Update service setup for my pages infra * chore: charts update dirty files * Remove from infra * undo rename --------- Co-authored-by: albinagu <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> Co-authored-by: Ásdís Erna Guðmundsdóttir <[email protected]> Co-authored-by: Þorkell Máni Þorkelsson <[email protected]> Co-authored-by: Svanhildur Einarsdóttir <[email protected]> Co-authored-by: Kristofer <[email protected]> Co-authored-by: helgifr <[email protected]> Co-authored-by: hfhelgason <[email protected]> Co-authored-by: veronikasif <[email protected]> Co-authored-by: Rafn Árnason <[email protected]> Co-authored-by: andes-it <[email protected]> Co-authored-by: Rúnar Vestmann <[email protected]> Co-authored-by: mannipje <[email protected]> Co-authored-by: unakb <[email protected]> Co-authored-by: juni-haukur <[email protected]> Co-authored-by: birkirkristmunds <[email protected]> Co-authored-by: Kristján Albert <[email protected]>
Summary by CodeRabbit
New Features
Documentation
Tests