Skip to content

test: unit & integration testing improvements#213

Merged
goosewobbler merged 14 commits intomainfrom
testing-review
Mar 23, 2026
Merged

test: unit & integration testing improvements#213
goosewobbler merged 14 commits intomainfrom
testing-review

Conversation

@goosewobbler
Copy link
Copy Markdown
Contributor

No description provided.

- Modified assertions in `bridge.spec.ts` to expect `client.close()` to resolve to `undefined` instead of not throwing an error.
- Updated error messages in `getDebuggerEndpoint` tests in `bridge.spec.ts` to be more specific.
- Changed `describe` blocks in various test files from async to synchronous for consistency.
- Removed outdated test files and added new tests for mock operations to enhance coverage and maintainability.
- Added new tests in `launcher.spec.ts` to verify behavior when only `appEntryPoint` is provided and to handle unexpected errors from `getBinaryPath`.
- Introduced tests in `pathResolver.spec.ts` for `getElectronBinaryPath`, covering successful resolution, package.json not found, and binary path resolution failures.
- Improved mock implementations in `mock.spec.ts` and `triggerDeeplink.spec.ts` to ensure accurate behavior and error handling during tests.
- Updated assertions and added caching logic for userDataDir detection in `triggerDeeplink.spec.ts` to enhance reliability.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 20, 2026

Greptile Summary

This PR is a broad test quality pass across electron-service, electron-cdp-bridge, native-spy, and native-utils. It consolidates three nearly-identical mock-operation spec files into a single describe.each-parameterised suite, fixes several antipatterns (improper async on describe/beforeEach, overly-aggressive vi.resetAllMocks() in afterEach, imprecise resolves.not.toThrowError() assertions), adds a local custom-matchers.d.ts so per-file /// <reference> directives are no longer needed, and substantially increases edge-case coverage in launcher, mock, pathResolver, service, triggerDeeplink, and native-spy.

Key changes:

  • clearAllMocks.spec.ts, resetAllMocks.spec.ts, restoreAllMocks.spec.ts deleted and replaced by the parameterised mockOperations.spec.ts, which also switches afterEach to vi.clearAllMocks() (addresses prior review concern)
  • Three resolves.not.toThrowError() calls in bridge.spec.ts (cdp-bridge) replaced with the correct resolves.toBeUndefined()
  • describe('connect', async () => …) and describe('Multiremote', async () => …) corrected to synchronous callbacks
  • @types/vitest/index.d.ts drops export * from 'vitest' (was the source of triple-slash reference proliferation); a mirroring custom-matchers.d.ts is added locally and wired into the test tsconfig.json; the two files now both augment vitest.Assertion — TypeScript merges them without error, but it is worth noting the slight inconsistency: the root file uses Assertion<T> (no default) while the local file uses Assertion<T = unknown>
  • New service.spec.ts test should reject when waitUntilWindowAvailable fails mocks the function via a dynamic import taken after vi.resetModules() runs in beforeEach; this works if Vitest's live-binding proxy routes the service's runtime call to the freshly-created mock instance — worth keeping an eye on if the test ever starts spuriously passing/failing

Confidence Score: 5/5

  • Test-only PR with no production code changes; safe to merge.
  • All changes are in test files. Prior review concerns (incompatible type annotations, unnecessary async on beforeEach, vi.resetAllMocks() aggressiveness) are addressed in the new mockOperations.spec.ts. The two minor observations — duplicate vitest.Assertion augmentation and the vi.resetModules() + dynamic-import mock isolation pattern — are both low-risk and non-blocking: the first is a harmless TypeScript merge, and the second is consistent with how the rest of the file already uses vi.resetModules().
  • packages/electron-service/test/service.spec.ts — the new waitUntilWindowAvailable failure test relies on Vitest live-binding semantics after vi.resetModules(); and @types/vitest/index.d.ts + packages/electron-service/test/custom-matchers.d.ts for the minor Assertion<T> default inconsistency.

Important Files Changed

Filename Overview
packages/electron-service/test/commands/mockOperations.spec.ts New file consolidating clearAllMocks, resetAllMocks, restoreAllMocks tests into a single parameterized describe.each suite; fixes vi.resetAllMocks() → vi.clearAllMocks() in afterEach and removes unnecessary async from beforeEach, addressing prior review comments.
@types/vitest/index.d.ts Replaced export * from 'vitest' with import 'vitest', removed default type from Assertion<T>; the same Assertion augmentation now also exists in custom-matchers.d.ts — harmless due to TS merging but redundant.
packages/electron-service/test/custom-matchers.d.ts New local type declaration file for electron-service tests; duplicates the vitest.Assertion augmentation from the root @types/vitest/index.d.ts, though both are harmlessly merged by TypeScript.
packages/electron-service/test/service.spec.ts Removed awkward self-mock of service.js for waitUntilWindowAvailable; adds new tests for CDP bridge degradation, fuse warning, and waitUntilWindowAvailable failure; the failure test dynamically imports @wdio/native-utils after vi.resetModules() — works if Vitest live-binds the fresh instance to the service's runtime call.
packages/electron-service/test/commands/triggerDeeplink.spec.ts Adds userData auto-detection tests and refactors platform mutation into beforeEach/afterEach; tests are well-isolated and cover happy-path, caching, skip, and failure-graceful scenarios.
packages/electron-service/test/mock.spec.ts Removes dynamic import workaround for createClassMock; new granular tests cover prototype methods, __constructor name, update() sync, mockRestore(), getMockName(), and non-class error.
packages/electron-service/test/launcher.spec.ts Adds tests for appEntryPoint-only path, unexpected getBinaryPath error wrapping, builder tool suggestion, and fixes async on describe('Multiremote').
packages/native-spy/test/spy.spec.ts Standardises test names to "should …" convention and adds substantial new coverage: mockResolvedValueOnce, mockRejectedValueOnce, mockRestore, throw-result tracking, instances, invocationCallOrder, queue ordering, context tracking, and withImplementation error recovery.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[mockOperations.spec.ts] -->|describe.each| B{fn parameter}
    B --> C[clearAllMocks]
    B --> D[resetAllMocks]
    B --> E[restoreAllMocks]
    C --> F[test: operate on all mocks]
    D --> F
    E --> F
    C --> G[test: operate on specific API]
    D --> G
    E --> G
    F --> H[assert: mockMethod called on all entries]
    G --> I[assert: mockMethod called only for matching API]

    J[service.spec.ts] --> K[vi.mock @wdio/native-utils via mocks/native-utils.ts]
    K --> L[waitUntilWindowAvailable = vi.fn]
    J --> M[beforeEach: vi.resetModules + vi.clearAllMocks]
    M --> N[test: should reject when waitUntilWindowAvailable fails]
    N --> O[dynamic import @wdio/native-utils]
    O --> P[mockRejectedValueOnce Window not found]
    P --> Q[instance.before rejects with Window not found]
Loading

Reviews (5): Last reviewed commit: "test(native-spy): add new test cases for..." | Re-trigger Greptile

Comment thread packages/electron-service/test/commands/mockOperations.spec.ts Outdated
Comment thread packages/electron-service/test/commands/mockOperations.spec.ts Outdated
goosewobbler and others added 7 commits March 21, 2026 10:44
- Modified the command execution in `build-package.ts` to use `cmd.exe` on Windows and `pnpm` on other platforms, ensuring proper invocation of the build process.
- Adjusted arguments for the spawn function to accommodate the changes in command structure, enhancing compatibility across different operating systems.
Fix pre-existing TS errors: vitest module augmentation shadowing, wrong
import source for waitUntilWindowAvailable, missing Result type narrowing
guards, and mismatched Assertion type parameters.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… and pathResolver

Add tests for execute params/callback in mockAll, class mock path in
mock command, and getElectronBinaryPath resolution in pathResolver.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Refine the type definitions for mocked functions in the mockOperations test suite to allow for both function return types and string return types, enhancing type safety and clarity in the tests.
Removed redundant mock definitions for appBuildInfo, binaryPath, and electronVersion, simplifying the test setup in pathResolver.spec.ts. This change enhances clarity and maintainability of the test suite.
Replace inline Record union type with a named MockApiMethod type that
explicitly declares getMockName, making the required property visible
at the type level.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread packages/electron-service/test/commands/mockOperations.spec.ts
Added multiple test cases to improve coverage for mock functions, including scenarios for `mockResolvedValueOnce`, `mockRejectedValueOnce`, `mockRestore`, and context tracking. These additions ensure better validation of mock behavior and state management in various use cases.
…PackageUpSync

Enhanced tests for `readPackageUp` and `readPackageUpSync` to validate functionality when a URL is provided as the `cwd`. This includes checks for both asynchronous and synchronous methods, ensuring that the package JSON is correctly read from the specified URL. Additionally, improved error handling tests in `selectExecutable` to cover scenarios with empty paths and errors without message properties.
Replaced `vi.resetAllMocks()` with `vi.clearAllMocks()` in the mock operations test suite to ensure a more precise reset of mock states after each test, improving test isolation and reliability.
…avior

Updated test descriptions in `spy.spec.ts` to follow a consistent format, enhancing clarity and readability. Each test now begins with "should" to clearly define the expected behavior of the mock functions.
…tracking

Introduced additional test scenarios in `spy.spec.ts` to enhance coverage for mock functions. New tests include tracking results for queued implementations that throw errors and verifying the tracking of instances, ensuring comprehensive validation of mock behavior.
@goosewobbler goosewobbler changed the title test(electron): unit & integration testing improvements test: unit & integration testing improvements Mar 23, 2026
@goosewobbler goosewobbler merged commit 3f55fb9 into main Mar 23, 2026
80 checks passed
@goosewobbler goosewobbler deleted the testing-review branch March 23, 2026 11:10
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.

1 participant