Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 1, 2025

Overview

Refactored src/find-in-page/__tests__/index.test.js to follow the recommended testing pattern by using real settings instead of mocked settings module. This aligns with the patterns established in src/tray/__tests__/index.test.js and src/tab-manager/__tests__/index.test.js.

Changes

Before (Mocked Settings)

beforeEach(() => {
  jest.resetModules();
  jest.mock('electron', () => require('../../__tests__').mockElectronInstance());
  electron = require('electron');
  baseWindow = electron.baseWindowInstance;
  electron.WebContentsView = jest.fn(() => require('../../__tests__').mockWebContentsViewInstance());
  eventBus = electron.ipcMain;
  // Always mock settings unless we want to overwrite the real settings file !
  jest.mock('../../settings');
  require('../../settings').loadSettings.mockImplementation(() => ({
    trayEnabled: true
  }));
  require('../../settings').openSettingsDialog = jest.requireActual('../../settings').openSettingsDialog;
  jest.spyOn(require('../../user-agent'), 'initBrowserVersions')
    .mockImplementation(() => Promise.resolve({}));
  main = require('../../main');
  main.init();
});

After (Real Settings)

beforeEach(async () => {
  jest.resetModules();
  await require('../../__tests__').testSettings();
  jest.mock('electron', () => require('../../__tests__').mockElectronInstance());
  electron = require('electron');
  baseWindow = electron.baseWindowInstance;
  electron.WebContentsView = jest.fn(() => require('../../__tests__').mockWebContentsViewInstance());
  eventBus = electron.ipcMain;
  jest.spyOn(require('../../user-agent'), 'initBrowserVersions')
    .mockImplementation(() => Promise.resolve({}));
  main = require('../../main');
  main.init();
});

Benefits

  • Simpler code: Removed 5 lines of complex mocking setup
  • Black-box testing: Tests now verify actual integrated behavior rather than implementation details
  • Better isolation: Each test suite uses real settings with temporary directories that are automatically cleaned up
  • Consistency: Follows the same pattern as other recently refactored tests
  • Maintainability: Easier to read and understand without mock complexity

Testing

✅ All 17 find-in-page tests pass
✅ All 627 tests in the full test suite pass
✅ No linting errors
✅ Test structure remains unchanged

Original prompt

The tests in src/find-in-page/tests/index.test.js use mocked settings module.
This is no longer the recommended practice in this repository.
We are now using real settings to ensure integrated behavior and to avoid tests dealing with internal implementations (black-box testing preferred over white-box testing).
Refactor this test suite.
You can check already refactored tests in:

  • src/tray/tests/index.test.js
  • src/tab-manager/tests/index.test.js

The current test structre should remain untouched. The test should be simpler to read and have less complexity.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: manusa <488391+manusa@users.noreply.github.com>
Copilot AI changed the title The tests in src/find-in-page/tests/index.test.js use mocked settings module. This is no longer the recommended practice in this repository. We are now using real settings to ensure integrated behavior and to avoid tests dealing with internal implement... Refactor find-in-page tests to use real settings instead of mocks Oct 1, 2025
Copilot AI requested a review from manusa October 1, 2025 05:28
Copilot finished work on behalf of manusa October 1, 2025 05:28
@manusa manusa added this to the 0.1.0 milestone Oct 1, 2025
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 1, 2025

Copy link
Owner

@manusa manusa left a comment

Choose a reason for hiding this comment

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

LGTM, thx!

@manusa manusa marked this pull request as ready for review October 1, 2025 07:13
Copilot AI review requested due to automatic review settings October 1, 2025 07:13
Copy link
Contributor

Copilot AI left a 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 refactors the find-in-page test suite to use real settings instead of mocked settings, following the repository's recommended testing pattern for better integration testing and maintainability.

  • Removes complex mocking setup for the settings module
  • Adopts real settings with temporary directories for test isolation
  • Aligns with patterns established in other recently refactored test suites

@manusa manusa merged commit c94c540 into main Oct 1, 2025
10 checks passed
@manusa manusa deleted the copilot/fix-ddb63007-0eef-4e0f-a801-7df51c7e8186 branch October 1, 2025 07:13
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.

2 participants