-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fix: upgraded msw to latest and all jest packages to latest #7399
base: main
Are you sure you want to change the base?
Conversation
Welcome to the SigNoz community! Thank you for your first pull request and making this project better. 🤗 |
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.
👍 Looks good to me! Reviewed everything up to 18ef654 in 2 minutes and 24 seconds
More details
- Looked at
1142
lines of code in28
files - Skipped
1
files when reviewing. - Skipped posting
15
drafted comments based on config settings.
1. frontend/src/pages/WorkspaceLocked/WorkspaceLocked.test.tsx:18
- Draft comment:
The tests look comprehensive. In the first test, the render is wrapped in act() (L18-20); consider removing the explicit act wrapper since render() from testing-library is already wrapped in act. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
2. frontend/src/container/LogsExplorerList/styles.ts:7
- Draft comment:
InfinityWrapperStyled sets two conflicting heights (40rem !important followed by 100%). Consider removing or consolidating these so that the intended height is clear. - Reason this comment was not posted:
Comment was on unchanged code.
3. frontend/src/container/GridTableComponent/index.tsx:140
- Draft comment:
Avoid inline style objects for dynamic threshold formatting. Consider moving the dynamic styling into a styled component or using conditional class names to improve maintainability. - 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.
4. frontend/package.json:7
- Draft comment:
There's a potential typographical inconsistency in the i18n script: the key is named 'i18n:generate-hash' but the file being executed is './i18-generate-hash.js'. Consider renaming the file to './i18n-generate-hash.js' (or adjusting the script) for consistency. - 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.
5. frontend/src/components/Spinner/styles.ts:10
- Draft comment:
Typographical error: The component is named 'SpinerStyle'. It seems like it should be 'SpinnerStyle'. Please update the naming to match the intended spelling. - Reason this comment was not posted:
Comment was on unchanged code.
6. frontend/src/container/AllAlertChannels/__tests__/CreateAlertChannel.test.tsx:11
- Draft comment:
Typo detected in the imported variable name: 'pagerDutyDescriptionDefaultVaule'. Please correct it to 'pagerDutyDescriptionDefaultValue'. - 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. frontend/src/container/AllAlertChannels/__tests__/CreateAlertChannel.test.tsx:76
- Draft comment:
Typo found in the test description: 'Recepient label, input, and help text' should be updated to 'Recipient label, input, and help text'. - 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.
8. frontend/src/container/LogsExplorerViews/tests/LogsExplorerViews.test.tsx:22
- Draft comment:
Typo: It looks like 'lodsQueryServerRequest' might be a typo and should probably be renamed to 'logsQueryServerRequest' to match the context more clearly. - Reason this comment was not posted:
Comment was on unchanged code.
9. frontend/src/container/NewDashboard/DashboardDescription/__tests__/DashboardDescription.test.tsx:35
- Draft comment:
Consider adjusting the phrasing in the test description on line 35. For example, instead of 'unlock dashboard should be disabled for integrations created dashboards', it may be clearer to use 'unlock dashboard should be disabled for integration-created dashboards'. - 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.
10. frontend/src/container/NewDashboard/DashboardDescription/__tests__/DashboardDescription.test.tsx:68
- Draft comment:
Consider revising the test description on line 68 for clarity. For example, instead of 'unlock dashboard should not be disabled for non integration created dashboards', consider using 'unlock dashboard should not be disabled for non-integration-created dashboards'. - 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.
11. frontend/src/container/ServiceApplication/ServiceMetrics/ServiceMetrics.test.tsx:28
- Draft comment:
Typo detected: In the test name 'should not render if the data is not prsent', the word 'prsent' seems to be a typo. It should be corrected to 'present'. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
While this is a real typo, it's a very minor issue in a test file name that doesn't affect functionality. The rules state not to make comments that are obvious or unimportant. Test names are internal to the codebase and don't affect user-facing functionality. However, maintaining clean code includes having correct spelling in test names for readability and professionalism.
The typo could make the test slightly harder to find using text search. However, it's such a minor issue that it might not be worth the overhead of a PR comment and review cycle.
While the typo is real, fixing it would require a round trip in the PR process for very little benefit. The test name is still completely understandable despite the typo.
Delete this comment as it points out an unimportant issue that doesn't affect functionality and would create unnecessary review overhead.
12. frontend/src/pages/DashboardsListPage/__tests__/DashboardListPage.test.tsx:48
- Draft comment:
Typo detected: It appears '/dashbords' is used instead of '/dashboards'. Please correct the URL path to avoid confusion. - 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.
13. frontend/src/pages/DashboardsListPage/__tests__/DashboardListPage.test.tsx:87
- Draft comment:
Typo in comment: The line '// change the sort by order and dashboards list ot be updated accordingly' contains 'ot' which should be 'to'. Please update the comment for clarity. - 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.
14. frontend/src/pages/TracesExplorer/__test__/TracesExplorer.test.tsx:480
- Draft comment:
Typo found in a comment: 'covererd' should be spelled as 'covered'. - 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.
15. frontend/src/pages/TracesExplorer/__test__/TracesExplorer.test.tsx:431
- Draft comment:
Typo found in a comment: 'reset id done' should be corrected to 'reset is done'. - 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_f2JXZOYiUXKgG9E7
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Upgraded msw to latest
2.7.3
and made relevant code changes.This led to cascade of updates in jest packages,
Which led to upgrading typescript to
5.8.2
.Fixed TS issues after the upgrade.
Important
Upgrade
msw
,jest
, and related packages, update TypeScript, and modify code for compatibility with newmsw
API.msw
to2.7.3
andjest
to29.7.0
inpackage.json
.typescript
to5.8.2
.babel-jest
,@jest/globals
,@types/jest
,ts-jest
,jest-environment-jsdom
,jest-fixed-jsdom
.rest
withhttp
andHttpResponse
inmsw
handlers inAPIKeys.test.tsx
,CreateAlertChannel.test.tsx
,handlers.ts
, and 10 other test files.children
andclassName
props to styled components instyles.ts
files across multiple components.testEnvironment
tojest-fixed-jsdom
injest.config.ts
.uuid
totransformIgnorePatterns
injest.config.ts
.This description was created by
for 18ef654. It will automatically update as commits are pushed.