proposal(7): Vitest + MSW + Playwright + CI matrix#19
Open
christian-byrne wants to merge 1 commit intoComfy-Org:mainfrom
Open
proposal(7): Vitest + MSW + Playwright + CI matrix#19christian-byrne wants to merge 1 commit intoComfy-Org:mainfrom
christian-byrne wants to merge 1 commit intoComfy-Org:mainfrom
Conversation
Add testing infrastructure proposal with working PoC: - Vitest config (Node environment for main-process code) - 41 passing unit tests across 5 files: - lib/util.js: parseUrl, formatTime, parseArgs (16 tests) - lib/i18n.js: t(), param substitution, fallback (7 tests) - installations.js: full CRUD + reorder (10 tests) - sources/git.js: buildInstallation, metadata (4 tests) - sources/standalone.js: buildInstallation, getDefaults (4 tests) - Module._load shim for mocking electron in CJS code - Playwright config placeholder for future E2E tests - GitHub Actions test.yml with 3-platform matrix - Proposal document with testability matrix and roadmap Key technical finding: Vitest does not intercept CJS require() calls, so electron must be shimmed via Module._load override. This workaround is well-documented and will be unnecessary after ESM migration (Proposal Comfy-Org#1). Amp-Thread-ID: https://ampcode.com/threads/T-019c7926-be08-704f-b583-1c5167315271
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposal #7: Testing Infrastructure
What
Add a complete testing strategy from zero: Vitest for unit tests, Playwright config for future E2E, and a GitHub Actions CI matrix that runs tests on all 3 platforms.
PoC Deliverables
lib/util.js—parseUrl,formatTime,parseArgs(16 tests)lib/i18n.js—t(), param substitution, locale fallback (7 tests)installations.js— full CRUD: add, remove, update, get, reorder (10 tests)sources/git.js—buildInstallation, source metadata (4 tests)sources/standalone.js—buildInstallation,getDefaults(4 tests).github/workflows/test.ymlwith ubuntu/windows/macos matrix.github/proposals/proposal-testing-ci.mdwith full testability matrixKey Technical Finding
Vitest 4.x does not intercept CJS
require()calls (vitest-dev/vitest#3134). To mockelectronfor modules likeinstallations.js → lib/paths.js → require('electron'), we overrideModule._loadviavi.hoisted(). This is the standard community workaround and will become unnecessary after ESM migration.What's NOT Included (future work)
@playwright/testdependency (config-only placeholder)Run it
See
.github/proposals/proposal-testing-ci.mdfor the full proposal with tradeoffs and roadmap.