chore: Fix mocks application in app layout tests #3277
Conversation
| @@ -1,9 +1,9 @@ | |||
| // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
| /* eslint simple-import-sort/imports: 0 */ | |||
There was a problem hiding this comment.
Primary reason for this change. We do not have side-effects in import order anymore, so we can remove this eslint rule exception
| jest.mock('../../../lib/components/internal/hooks/use-mobile', () => ({ | ||
| useMobile: jest.fn().mockReturnValue(true), | ||
| })); |
There was a problem hiding this comment.
Here and in many places below – removed direct mock applications, because describeEachAppLayout helper takes care of setting up the correct environment
|
|
||
| // in our ResizeObserver mock resolves into mobile mode | ||
| test('should render mobile mode by default', () => { | ||
| test('does not render mobile mode by default', () => { | ||
| const { wrapper } = renderComponent(<AppLayout />); | ||
| expect(wrapper.findByClassName(mobileStyles['mobile-bar'])).toBeTruthy(); | ||
| expect(wrapper.findByClassName(mobileStyles['mobile-bar'])).toBeFalsy(); | ||
| }); |
There was a problem hiding this comment.
The comment was wrong, there is no resize observer mock driving this.
It was useMobile: jest.fn().mockReturnValue(true) configuration. When this removed, we can actually see that the default JSDOM rendering is the desktop variant
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3277 +/- ##
==========================================
- Coverage 96.44% 96.42% -0.03%
==========================================
Files 791 791
Lines 22569 22573 +4
Branches 7733 7794 +61
==========================================
- Hits 21766 21765 -1
- Misses 750 801 +51
+ Partials 53 7 -46 ☔ View full report in Codecov by Sentry. |
| jest.mock('@cloudscape-design/component-toolkit/internal', () => ({ | ||
| ...jest.requireActual('@cloudscape-design/component-toolkit/internal'), | ||
| isMotionDisabled: jest.fn().mockReturnValue(true), | ||
| })); |
There was a problem hiding this comment.
Removed these mocks, because they do not affect the result anyhow. If we need the disabled motion in our tests, we should use our own official utility: disableMotion(true)
| const forceMobileMode = (globalThis as any)[forceMobileModeSymbol]; | ||
| if (typeof forceMobileMode !== 'undefined') { | ||
| return forceMobileMode; | ||
| } |
There was a problem hiding this comment.
This follows our VR flags idea: cloudscape-design/component-toolkit#79
Currently this is used only internally, but we might make it our official test util to allow our consumers write unit tests on mobile use-cases
| // remove after this is released: https://github.com/cloudscape-design/component-toolkit/pull/118 | ||
| document.body.classList.remove('awsui-visual-refresh'); |
There was a problem hiding this comment.
just-boris
requested review from
gethinwebster
and removed request for
a team
2 participants
Description
Remove a lot of
jest.mockcallsRelated links, issue #, if available: n/a
How has this been tested?
Test-only changes
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.