Skip to content

Conversation

@ziyang-lin-404
Copy link
Contributor

Context

The previous PR added 5 test suites to testPathIgnorePatterns due to complex mocking requirements for @scality/data-browser-library. This PR fixes those tests and removes the ignore patterns.

Problem

  • 5 test suites were skipped because they required mocks for @scality/data-browser-library hooks
  • MSW-based mocking didn't work properly with the library's internal implementation
  • Clipboard API tests failed due to async timing issues with userEvent

Solution

  • Added global mock for @scality/data-browser-library in jestSetupAfterEnv.tsx
  • Replaced MSW server mocking with direct hook mocks
  • Changed clipboard tests to use jest.spyOn with synchronous fireEvent
  • Fixed validation error message text to match actual Joi output

Changes

Modified Files

File Changes
jest.config.js Removed testPathIgnorePatterns for 5 test suites
jestSetupAfterEnv.tsx Added global jest.mock('@scality/data-browser-library') and clipboard mock implementation
useGetS3ServicePoint.test.ts Changed from jest.mock to jest.spyOn pattern for proper module mocking
VeeamCapacityOverviewRow.test.tsx Replaced MSW with direct useGetBucketTagging and useGetObject hook mocks
VeeamCapacityModal.test.tsx Replaced MSW with direct usePutObject hook mock; fixed validation message text
ISVSummary.test.tsx Changed useGetS3ServicePoint to jest.spyOn; switched clipboard tests from userEvent to fireEvent
SelectAccountIAMRole.test.tsx Added mock for DataServiceRoleProvider to provide assumedRole immediately; simplified MSW handler

Issues Encountered and Solutions

Issue 1: jest.mock Not Working for Partial Module Mocks

Cause: jest.mock with factory function couldn't properly mock specific exports while keeping others.

Fix: Used jest.spyOn on imported module objects:

import * as accountsModule from '../../domain/business/accounts';
jest.spyOn(accountsModule, 'useAccountsLocationsAndEndpoints').mockReturnValue({...});

Issue 2: MSW Not Intercepting data-browser-library Requests

Cause: The library uses internal S3 client that doesn't go through standard fetch/XHR that MSW intercepts.

Fix: Mock the hooks directly:

const mockUseGetBucketTagging = useGetBucketTagging as jest.Mock;
mockUseGetBucketTagging.mockReturnValue({ data: {...}, status: 'success' });

Issue 3: Clipboard Tests Failing with userEvent

Cause: userEvent.click is async and the clipboard mock's state wasn't properly synchronized.

Fix: Use synchronous fireEvent.click and jest.spyOn assertions:

const writeTextSpy = jest.spyOn(navigator.clipboard, 'writeText');
fireEvent.click(copyButton);
expect(writeTextSpy).toHaveBeenLastCalledWith(expectedText);

Issue 4: SelectAccountIAMRole Stuck at Loading State

Cause: DataServiceRoleProvider was waiting for async assumeRole call that never completed in test environment.

Fix: Mock DataServiceRoleProvider to provide assumedRole context immediately:

jest.mock('../../DataServiceRoleProvider', () => ({
  default: ({ children }) => (
    <actual._DataServiceRoleContext.Provider value={{ assumedRole: {...} }}>
      {children}
    </actual._DataServiceRoleContext.Provider>
  ),
}));

Issue 5: Validation Message Mismatch

Cause: Test expected "larger than" but Joi outputs "greater than".

Fix: Updated test assertion to match actual Joi validation message.

Test Coverage Progress

Metric Before After
Ignored test suites 5 0
Tests in ignored suites 41 0
Total passing tests 563 604

Testing

  1. All 5 previously ignored test suites now pass (40 tests + 1 pre-existing skip)
  2. No regressions in other tests

…ignores and enhance tests with mock implementations for clipboard interactions and data-browser-library
@bert-e
Copy link
Contributor

bert-e commented Dec 24, 2025

Hello ziyang-lin-404,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options
name description privileged authored
/after_pull_request Wait for the given pull request id to be merged before continuing with the current one.
/bypass_author_approval Bypass the pull request author's approval
/bypass_build_status Bypass the build and test status
/bypass_commit_size Bypass the check on the size of the changeset TBA
/bypass_incompatible_branch Bypass the check on the source branch prefix
/bypass_jira_check Bypass the Jira issue check
/bypass_peer_approval Bypass the pull request peers' approval
/bypass_leader_approval Bypass the pull request leaders' approval
/approve Instruct Bert-E that the author has approved the pull request. ✍️
/create_pull_requests Allow the creation of integration pull requests.
/create_integration_branches Allow the creation of integration branches.
/no_octopus Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
/unanimity Change review acceptance criteria from one reviewer at least to all reviewers
/wait Instruct Bert-E not to run until further notice.
Available commands
name description privileged
/help Print Bert-E's manual in the pull request.
/status Print Bert-E's current status in the pull request TBA
/clear Remove all comments from Bert-E from the history TBA
/retry Re-start a fresh build TBA
/build Re-start a fresh build TBA
/force_reset Delete integration branches & pull requests, and restart merge process from the beginning.
/reset Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

@ziyang-lin-404 ziyang-lin-404 marked this pull request as ready for review December 24, 2025 13:46
@bert-e
Copy link
Contributor

bert-e commented Dec 24, 2025

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

Copy link
Contributor

@hervedombya hervedombya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ziyang-lin-404
Copy link
Contributor Author

/approve

@bert-e
Copy link
Contributor

bert-e commented Dec 24, 2025

I have successfully merged the changeset of this pull request
into targetted development branches:

  • ✔️ development/4.2

The following branches have NOT changed:

  • development/1.4
  • development/1.5
  • development/1.6
  • development/2.0
  • development/2.1
  • development/2.2
  • development/3.0
  • development/3.1
  • development/3.2
  • development/4.0
  • development/4.1

Please check the status of the associated issue None.

Goodbye ziyang-lin-404.

The following options are set: approve

@bert-e bert-e merged commit befa80d into development/4.2 Dec 24, 2025
9 checks passed
@bert-e bert-e deleted the improvement/integrate-library-test-fix branch December 24, 2025 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants