-
-
Notifications
You must be signed in to change notification settings - Fork 69
chore: add correct test case for #409 #441
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
|
WalkthroughThis change removes the entire Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant Resolver
participant PnPEnvironment
TestRunner->>Resolver: Request resolve('@atlaskit/pragmatic-drag-and-drop/combine')
Resolver->>PnPEnvironment: Locate module in pnp cache
PnPEnvironment-->>Resolver: Return resolved path to .d.ts file
Resolver-->>TestRunner: Return found result with resolved path
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
yarn install v1.22.22 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (8)
📒 Files selected for processing (7)
💤 Files with no reviewable changes (5)
⏰ Context from checks skipped due to timeout of 90000ms (6)
🔇 Additional comments (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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.
Pull Request Overview
This PR updates the test suite to correctly address issue #409 by adding a new unit test and cleaning up outdated configurations from the end-to-end tests.
- Adds a unit test case for verifying the resolution of '@atlaskit/pragmatic-drag-and-drop/combine'
- Removes obsolete export and ESLint configuration from nestedPackageJson tests
Reviewed Changes
Copilot reviewed 10 out of 15 changed files in this pull request and generated no comments.
File | Description |
---|---|
tests/unit/unit.spec.ts | Added a new unit test using an inline snapshot to validate the resolver. |
tests/e2e/nestedPackageJson/test.ts | Removed unused export related to the outdated package reference. |
tests/e2e/nestedPackageJson/.eslintrc.cjs | Removed obsolete ESLint configuration pointing to the base config. |
Files not reviewed (5)
- tests/e2e/snapshots/e2e.spec.ts.snap: Language not supported
- tests/e2e/nestedPackageJson/.gitignore: Language not supported
- tests/e2e/nestedPackageJson/package.json: Language not supported
- tests/e2e/nestedPackageJson/tsconfig.json: Language not supported
- tests/unit/pnp/package.json: Language not supported
Comments suppressed due to low confidence (1)
tests/unit/unit.spec.ts:40
- [nitpick] Consider adding a brief comment above the snapshot assertion to clarify the reasoning behind expecting the resolver to return { "found": false } for '@atlaskit/pragmatic-drag-and-drop/combine'.
expect(
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #441 +/- ##
=======================================
Coverage 64.45% 64.45%
=======================================
Files 6 6
Lines 377 377
Branches 48 49 +1
=======================================
Hits 243 243
Misses 134 134 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
commit: |
size-limit report 📦
|
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.
Important
Looks good to me! 👍
Reviewed everything up to 87cf0f4 in 55 seconds. Click for details.
- Reviewed
86
lines of code in7
files - Skipped
8
files when reviewing. - Skipped posting
9
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. tests/e2e/__snapshots__/e2e.spec.ts.snap:64
- Draft comment:
Removed nestedPackageJson snapshot. Ensure related test files and references are removed to avoid stale snapshots. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. tests/e2e/nestedPackageJson/.eslintrc.cjs:1
- Draft comment:
Deleted file for nestedPackageJson ESLint config. Confirm that removal of nested package tests is intentional and does not break any dependent tests. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. tests/unit/pnp/package.json:6
- Draft comment:
Added dependency '@atlaskit/pragmatic-drag-and-drop'. Verify that the version (^1.5.2) matches what is expected in other parts of the project for consistency. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the author to verify the version of a dependency, which is related to dependencies. According to the rules, comments on dependencies should be ignored unless they are about specific version mismatches that need confirmation. This comment is asking for verification, which is not allowed.
4. tests/unit/unit.spec.ts:40
- Draft comment:
New snapshot expectation for '@atlaskit/pragmatic-drag-and-drop/combine' added. Ensure this matches the actual output and that the test file for resolution is updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to ensure that the snapshot matches the actual output and that the test file is updated. This falls under asking the author to ensure something is tested or verified, which is against the rules.
5. tests/e2e/__snapshots__/e2e.spec.ts.snap:59
- Draft comment:
Removal of the 'nestedPackageJson' snapshot appears intentional. Ensure that its removal aligns with the deletion of corresponding tests. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. tests/e2e/nestedPackageJson/.eslintrc.cjs:1
- Draft comment:
Deleted nestedPackageJson config file. Confirm that removal of this and related files (.gitignore, package.json, test.ts) is expected perimport/no-unresolved
unable to resolve@atlaskit/pragmatic-drag-and-drop
’s nested imports after upgrading 3.8.7 → 4.2.4 #409. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. tests/unit/pnp/package.json:6
- Draft comment:
Added '@atlaskit/pragmatic-drag-and-drop' dependency. Verify that version '^1.5.2' is correct and compatible with PnP resolution tests. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to verify the version of a dependency, which is against the rules. It is not making a specific suggestion or pointing out a specific issue with the code.
8. tests/unit/unit.spec.ts:41
- Draft comment:
New inline snapshot test for '@atlaskit/pragmatic-drag-and-drop/combine' added. Ensure the resolved path remains valid if dependency structure changes. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the author to ensure that the resolved path remains valid if the dependency structure changes. This is a request for the author to double-check something, which violates the rules.
9. tests/e2e/__snapshots__/e2e.spec.ts.snap:43
- Draft comment:
Typo noticed: The snapshot key 'multipleEslintrcs' might be intended to be 'multipleEslintrc'. Please verify and correct if necessary. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_hjXiR7xT85YmvUX1
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Important
Remove
nestedPackageJson
test case and update dependencies in unit tests.nestedPackageJson
test case frome2e.spec.ts.snap
.nestedPackageJson
related files:.eslintrc.cjs
,.gitignore
,package.json
, andtest.ts
.@atlaskit/pragmatic-drag-and-drop
todevDependencies
inpnp/package.json
.@atlaskit/pragmatic-drag-and-drop/combine
inunit.spec.ts
.This description was created by
for 87cf0f4. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Tests
Chores