diff --git a/src/main.js b/src/main.js index ca383a4..3fe3096 100644 --- a/src/main.js +++ b/src/main.js @@ -31,6 +31,7 @@ const { normalizeEol } = require('./git-update.cjs'); const { ensureAutocrlf, readTrunkInfo, collectDirtyFiles, discardChanges, updateToLatestTrunk } = require('./trunk-update'); const { openExternalUrl, ALLOWED_URL_SCHEMES } = require('./external-url'); const { deleteRegisteredSite } = require('./site-registry'); +const { getStore } = require('./settings-store'); const WORDPRESS_GIT_URL = 'https://github.com/WordPress/wordpress-develop.git'; @@ -124,31 +125,6 @@ function spawnRunner(runnerPath, args, { cwd, extraEnv = {} }) { }); } -let store; // initialized asynchronously due to ESM-only module -let storeReady = null; - -// The import starts on first use rather than at require time. Deferring it costs -// nothing — no handler can run before a window exists — and it keeps two things -// out of module load: a rejected promise nobody is awaiting yet (an unhandled -// rejection at startup, where the app has no way to report it), and the ESM -// loader pulling in `electron` behind `Module._load`'s back, which is what lets -// test/ipc-wiring.test.cjs require this file outside an Electron process. -async function getStore() { - if (!store) { - if (!storeReady) { - storeReady = import('electron-store').then((m) => { - const Store = m.default || m; - store = new Store({ - name: 'settings', - defaults: { sites: [], siteMeta: {} } - }); - }); - } - await storeReady; - } - return store; -} - function findAvailableDirName(rootDir, baseName) { const sanitizedBase = baseName || 'wordpress-develop-trunk'; let candidate = sanitizedBase; diff --git a/src/settings-store.js b/src/settings-store.js new file mode 100644 index 0000000..6ee0585 --- /dev/null +++ b/src/settings-store.js @@ -0,0 +1,36 @@ +// The one `electron-store` instance the app has, behind a seam tests can reach. +// +// `electron-store` is ESM-only, so main.js loaded it with a dynamic `import()`. +// That import is invisible to `Module._load`, which is what test/ipc-wiring.test.cjs +// uses to stand in for a module — so every handler that read the store was +// unreachable from that suite, and the delete handler's registry gate could be cut +// without failing anything (#145). Keeping the import in a module of its own means +// the harness replaces this file instead of trying to intercept the ESM loader. +// +// The import starts on first use rather than at require time. Deferring it costs +// nothing — no handler can run before a window exists — and it keeps a rejected +// promise nobody is awaiting yet out of module load, where the app has no way to +// report it. It also keeps the ESM loader from pulling in `electron` behind +// `Module._load`'s back, which is what lets that suite require main.js outside an +// Electron process at all. + +let store; +let storeReady = null; + +async function getStore() { + if (!store) { + if (!storeReady) { + storeReady = import('electron-store').then((m) => { + const Store = m.default || m; + store = new Store({ + name: 'settings', + defaults: { sites: [], siteMeta: {} } + }); + }); + } + await storeReady; + } + return store; +} + +module.exports = { getStore }; diff --git a/test/ipc-wiring.test.cjs b/test/ipc-wiring.test.cjs index 6052e3f..bd3590c 100644 --- a/test/ipc-wiring.test.cjs +++ b/test/ipc-wiring.test.cjs @@ -326,6 +326,107 @@ test('url:open refuses a file: address and opens an http one', async () => { assert.deepEqual(main.calls.openExternal, ['https://wordpress.org/']); }); +// The settings store is an ESM-only dependency loaded through a dynamic import, +// which `Module._load` cannot intercept — so it lives behind src/settings-store.js +// and this stands in for it. Values are held in a plain object the test can read +// back, because "the store was not written" is half of what the delete gate +// promises (#145). +function fakeSettingsStore(initial = {}) { + const values = { sites: [], siteMeta: {}, ...initial }; + const store = { + // A copy, like the real one: `conf` re-reads and re-deserializes the file on + // every `get`, so a handler that mutates what it read and never sets it back + // loses the write in the app. Returning the live object here would let that + // bug pass. + get: (key) => structuredClone(values[key]), + set: (key, value) => { values[key] = value; } + }; + return { values, stubs: { './settings-store': { getStore: async () => store } } }; +} + +// --- sites:delete -> src/site-registry.js -------------------------------- + +test('sites:delete asks site-registry whether the path may be removed', async () => { + const deleteRegisteredSite = spy(async () => true); + const settings = fakeSettingsStore({ sites: ['/sites/wp'] }); + const main = loadMain({ + stubs: { ...silentLogging(), ...settings.stubs, './site-registry': { deleteRegisteredSite } } + }); + + await main.invoke('sites:delete', '/sites/wp'); + + assert.equal(deleteRegisteredSite.calls.length, 1); + const [sitePath, options] = deleteRegisteredSite.calls[0]; + assert.equal(sitePath, '/sites/wp'); + // The module decides on the registry it is handed and acts through the + // callbacks: without the store's own `sites` array it would be deciding + // against nothing, and without the callbacks it could not act on its answer. + assert.deepEqual(options.sites, ['/sites/wp']); + assert.equal(typeof options.forget, 'function'); + assert.equal(typeof options.remove, 'function'); + assert.equal(typeof options.onRefused, 'function'); +}); + +// The end of the wire, with the real module and the real `fse.remove` in place, +// on real directories: this is the assertion #145 is about. It fails if the +// handler stops consulting site-registry, however it stops — deleted call, +// renamed export, or a bare `fse.remove(sitePath)` added beside it. +test('sites:delete removes a registered directory and refuses an unregistered one', async (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ipc-wiring-delete-')); + t.after(() => fs.rmSync(root, { recursive: true, force: true })); + const registered = path.join(root, 'registered'); + const unregistered = path.join(root, 'unregistered'); + for (const dir of [registered, unregistered]) { + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'src', 'index.php'), ' src/trunk-update.js ---------------------------------- + +test('site:status reports the trunk snapshot trunk-update read, not its own guess', async () => { + const readTrunkInfo = spy(async () => ({ trunkOid: 'abc123', trunkDate: '2026-01-01T00:00:00Z' })); + const settings = fakeSettingsStore({ sites: ['/sites/wp'], siteMeta: { '/sites/wp': { initialized: true } } }); + const main = loadMain({ + stubs: { ...silentLogging(), ...settings.stubs, './trunk-update': { readTrunkInfo } } + }); + + const status = await main.invoke('site:status', '/sites/wp'); + + assert.deepEqual(readTrunkInfo.calls, [['/sites/wp']]); + assert.equal(status.trunkOid, 'abc123'); + assert.equal(status.trunkDate, '2026-01-01T00:00:00Z'); + // Written through to siteMeta so the sidebar can render staleness dots from + // siteMeta alone, without per-site git I/O (#94). + assert.equal(settings.values.siteMeta['/sites/wp'].trunkOid, 'abc123'); +}); + // --- git:* -> src/trunk-update.js ---------------------------------------- test('git:worktree-dirty reports what trunk-update found, not its own guess', async () => { @@ -377,10 +478,10 @@ test('git:update-trunk hands the update to trunk-update and streams its log back test('sites:add normalizes line endings before adopting a directory', async () => { // Throwing ends the handler at its first delegation, which is the only thing // under test — and it has to end there. The next line is a store write, and - // reaching it would start `import('electron-store')`, whose own - // `import {app} from 'electron'` loads the real electron package through the - // ESM loader, out of reach of the hook. See the guard test below for why that - // must not happen. + // this test hands the harness no settings store, so reaching it would start + // the real `import('electron-store')`, whose own `import {app} from 'electron'` + // loads the real electron package through the ESM loader, out of reach of the + // hook. See the guard test below for why that must not happen. const ensureAutocrlf = spy(async () => { throw new Error('not a repository'); }); const main = loadMain({ stubs: { ...silentLogging(), './trunk-update': { ensureAutocrlf } } }); @@ -788,10 +889,11 @@ test('playground-web:stop ends the web server tree rather than signalling the ch // Two holes this closes, both of which reached the package on a cold checkout: // stubs are built by merging over the real module, so stubbing src/logging.js // means requiring it and it requires `electron` — that require has to happen -// with the hook already installed. And any handler allowed to reach `getStore()` -// starts `import('electron-store')`, which imports `electron` through the ESM -// loader, where `Module._load` does not apply and no hook can help. Tests stop -// short of the store instead. +// with the hook already installed. And a handler that reaches the real +// `getStore()` starts `import('electron-store')`, which imports `electron` +// through the ESM loader, where `Module._load` does not apply and no hook can +// help. That is what src/settings-store.js is a seam for: a test whose handler +// touches the store stubs it (fakeSettingsStore above), and the rest stop short. test('the harness never loads the real electron package', () => { loadMain({ stubs: silentLogging() }); @@ -813,6 +915,8 @@ const WIRED = new Set([ 'git:create-patch', 'git:save-patch', 'sites:add', + 'sites:delete', + 'site:status', 'npm:install', 'npm:run-script', 'npm:kill', @@ -833,7 +937,6 @@ const NO_DELEGATION = new Map([ ['sites:set-skip-init', 'electron-store write'], ['sites:mark-initialized', 'electron-store write'], ['sites:forget', 'electron-store write'], - ['sites:delete', 'electron-store write plus a directory removal'], ['sites:set-label', 'electron-store write'], ['dialog:choose-dir', 'opens the directory dialog'], ['playground-web:available', 'checks a path on disk'], @@ -857,7 +960,6 @@ const NO_DELEGATION = new Map([ // Channels that do delegate, but whose call sits behind something this harness // cannot stand in for yet. Each one is a known hole, not an oversight. const NOT_REACHABLE = new Map([ - ['site:status', 'reads electron-store before calling readTrunkInfo, and the store is a dynamic ESM import that Module._load cannot replace'], ['wordpress:setup', 'calls ensureAutocrlf and readTrunkInfo only after cloning wordpress-develop over the network'] ]);