Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 1, 2025

Refactored the test suite in src/main/__tests__/main-window-listeners.test.js to use real settings instead of mocks, following the repository's recommended testing practices.

Original issue description:

The tests in src/main/tests/main-window-listeners.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
  • src/find-in-page/tests/index.test.js

The current test structure should remain untouched.
The test should be simpler to read and have less complexity after refactoring.

Changes Made

  • Removed mock dependencies: Eliminated jest.mock('../../settings') and all mock implementations
  • Used real settings: Implemented testSettings() utility for isolated test environment with temporary directories
  • Simplified test code: Replaced require('../../settings').loadSettings.mockImplementation() with direct settings.updateSettings() calls
  • Made beforeEach async: Changed to async function since testSettings() returns a Promise

Testing

  • ✅ All 627 tests pass
  • ✅ Test structure remains intact (same describe blocks and test cases)
  • ✅ Code is simpler and more readable
  • ✅ Follows black-box testing approach over white-box testing
  • ✅ Consistent with other refactored tests (tray, tab-manager, find-in-page)

The refactoring reduces complexity and ensures the tests validate actual integrated behavior rather than mocked implementations.

Fixes #522

Original prompt

This section details on the original issue you should resolve

<issue_title>[TESTS] Refactor main-window-listeners to use real settings instead of mocks</issue_title>
<issue_description>The tests in src/main/tests/main-window-listeners.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
  • src/find-in-page/tests/index.test.js

The current test structure should remain untouched.
The test should be simpler to read and have less complexity after refactoring.</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes #522

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@manusa
Copy link
Owner

manusa commented Oct 1, 2025

@copilot, something went wrong and you stopped. Please retry

Co-authored-by: manusa <488391+manusa@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Oct 1, 2025

@copilot, something went wrong and you stopped. Please retry

Done! I've successfully refactored the main-window-listeners tests to use real settings instead of mocks (commit 1e8151a).

The refactoring:

  • Removed mock of settings module
  • Uses testSettings() for isolated test environment
  • Replaced mock implementation with direct settings.updateSettings() call
  • Test structure remains intact with same describe blocks and test cases
  • All 627 tests pass ✅

The code is now simpler and follows black-box testing approach as recommended.

Copilot AI changed the title [WIP] [TESTS] Refactor main-window-listeners to use real settings instead of mocks [TESTS] Refactor main-window-listeners to use real settings instead of mocks Oct 1, 2025
Copilot finished work on behalf of manusa October 1, 2025 08:07
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 08:14
Copilot AI review requested due to automatic review settings October 1, 2025 08:14
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 test suite in src/main/__tests__/main-window-listeners.test.js to eliminate mocked settings and use real settings instead, aligning with the repository's current testing practices that favor black-box testing over white-box testing.

  • Removed Jest mocks for the settings module to use real settings implementation
  • Replaced mock setup with testSettings() utility for isolated test environments
  • Simplified test code by replacing mock implementations with direct settings calls

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 1, 2025

@manusa manusa merged commit 667e3f0 into main Oct 1, 2025
15 of 18 checks passed
@manusa manusa deleted the copilot/fix-e82d0070-3def-4c27-8dca-30e9f9146adf branch October 1, 2025 10:43
@manusa manusa added this to the 0.1.0 milestone Oct 1, 2025
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.

[TESTS] Refactor main-window-listeners to use real settings instead of mocks

2 participants