Skip to content

Commit 03fbda6

Browse files
committed
Test-quality cleanup: tighten assertions, add expectError helper
Companion to the audit cleanup. Addresses every High and the highest- leverage Medium items from TEST_AUDIT_PLAN.md so the test suite catches real regressions instead of just confirming functions ran. CLI: - expectError(fn, match?) helper added to test/helpers/test-setup.ts; replaces the verbose try { await x; expect.fail(...) } catch {} pattern that could swallow the wrong error type. - code/deploy: tighten .calledOnce checks to verify args (instance, cartridges, code version) and afterOperation hook payload. - code/activate: assert PATCH path/body and reload toggle order (active → alternate → active) instead of just call counts. - auth/token: assert returned JSON shape and that ux.stdout was called with exactly the access token, not just .calledOnce. - mrt/env/var/push: positively assert that listEnvVars ran when asserting setBatchStub was *not* called. - cip/query: drop echo tautologies (result.sql === input); assert that the resolved SQL was passed to mockClient.query. - Promote sinon.stub(odsClient) and makeCommandThrowOnError out of 12 duplicated copies in sandbox tests into the canonical helpers. - Switch .to.equal(true|false) → .to.be.true|false where present. SDK: - logger.test.ts: drop 18 redundant `expect(logger).to.exist` lines (the followup `.to.be.a('function')` was the real check). MCP: - registry: new smoke test invokes a registered handler (sfnext_get_guidelines) end-to-end; previously only tool names were asserted. - theming-store: 25 weak `expect(guidance).to.exist` calls upgraded to `expect(guidance, '...').to.not.be.undefined` so failures point at the missing key. - figma generate-component: replace 7 `array.some(d => …).to.equal(true)` chains with `array.find(...)` + concrete property assertions so failures show what actually went wrong. mrt-utilities: - create-lambda-adapter-compression.test.ts: replace 39 fixed `setTimeout(50ms)` waits with the existing event-driven `stream.waitForEnd()`. Suite runtime drops from ~3s to ~1s. All 4,076 tests pass (+1 from the new MCP smoke test). Lint and typecheck clean.
1 parent 04fa47d commit 03fbda6

23 files changed

Lines changed: 220 additions & 301 deletions

File tree

packages/b2c-cli/test/commands/auth/token.test.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,12 @@ describe('auth token', () => {
4242

4343
const result = await command.run();
4444

45-
expect(strategy.getTokenResponse.calledOnce).to.equal(true);
46-
expect(result.accessToken).to.equal('token123');
47-
expect(result.scopes).to.have.lengthOf(1);
45+
expect(strategy.getTokenResponse.calledOnce).to.be.true;
46+
expect(result).to.deep.equal({
47+
accessToken: 'token123',
48+
expires: new Date('2025-01-01T00:00:00.000Z').toISOString(),
49+
scopes: ['scope1'],
50+
});
4851
});
4952

5053
it('prints raw token in non-JSON mode', async () => {
@@ -69,6 +72,6 @@ describe('auth token', () => {
6972

7073
await command.run();
7174

72-
expect(stdoutStub.calledOnce).to.equal(true);
75+
expect(stdoutStub.calledOnceWithExactly('token123')).to.be.true;
7376
});
7477
});

packages/b2c-cli/test/commands/cip/query.test.ts

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import {expect} from 'chai';
88
import sinon from 'sinon';
99
import CipQuery from '../../../src/commands/cip/query.js';
10-
import {createIsolatedConfigHooks, createTestCommand, runSilent} from '../../helpers/test-setup.js';
10+
import {createIsolatedConfigHooks, createTestCommand, expectError, runSilent} from '../../helpers/test-setup.js';
1111

1212
describe('cip query', () => {
1313
const hooks = createIsolatedConfigHooks();
@@ -42,8 +42,13 @@ describe('cip query', () => {
4242

4343
const result = await command.run();
4444

45+
// Verify the client received the resolved SQL — not just that the result
46+
// echoes the input unchanged, which would pass even if the command dropped it.
47+
expect(mockClient.query.calledOnce, 'CIP client should be invoked once').to.be.true;
48+
expect(mockClient.query.firstCall.args[0]).to.equal('SELECT 1 as num');
4549
expect(result.sql).to.equal('SELECT 1 as num');
4650
expect(result.columns).to.deep.equal(['num']);
51+
expect(result.rows).to.deep.equal([{num: 1}]);
4752
expect(result.rowCount).to.equal(1);
4853
});
4954

@@ -52,27 +57,19 @@ describe('cip query', () => {
5257

5358
const errorStub = sinon.stub(command, 'error').throws(new Error('Expected error'));
5459

55-
try {
56-
await command.run();
57-
expect.fail('Should have thrown');
58-
} catch {
59-
expect(errorStub.called).to.be.true;
60-
expect(errorStub.firstCall.args[0]).to.include('No SQL provided');
61-
}
60+
await expectError(() => command.run());
61+
expect(errorStub.called).to.be.true;
62+
expect(errorStub.firstCall.args[0]).to.include('No SQL provided');
6263
});
6364

6465
it('throws error when multiple SQL sources are provided', async () => {
6566
const command = await createCommand({file: 'query.sql'}, {sql: 'SELECT 1'});
6667

6768
const errorStub = sinon.stub(command, 'error').throws(new Error('Expected error'));
6869

69-
try {
70-
await command.run();
71-
expect.fail('Should have thrown');
72-
} catch {
73-
expect(errorStub.called).to.be.true;
74-
expect(errorStub.firstCall.args[0]).to.include('exactly one source');
75-
}
70+
await expectError(() => command.run());
71+
expect(errorStub.called).to.be.true;
72+
expect(errorStub.firstCall.args[0]).to.include('exactly one source');
7673
});
7774

7875
it('replaces FROM and TO placeholders in SQL', async () => {
@@ -120,7 +117,10 @@ describe('cip query', () => {
120117

121118
const result = (await runSilent(() => command.run())) as any;
122119

123-
expect(result.sql).to.equal('SELECT 1');
120+
expect(mockClient.query.calledOnce).to.be.true;
121+
expect(mockClient.query.firstCall.args[0]).to.equal('SELECT 1');
122+
expect(result.rowCount).to.equal(1);
123+
expect(result.columns).to.deep.equal(['num']);
124124
});
125125

126126
it('outputs CSV format when format flag is csv', async () => {
@@ -141,6 +141,8 @@ describe('cip query', () => {
141141

142142
const result = (await runSilent(() => command.run())) as any;
143143

144-
expect(result.sql).to.equal('SELECT 1');
144+
expect(mockClient.query.calledOnce).to.be.true;
145+
expect(mockClient.query.firstCall.args[0]).to.equal('SELECT 1');
146+
expect(result.rowCount).to.equal(1);
145147
});
146148
});

packages/b2c-cli/test/commands/code/activate.test.ts

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {expect} from 'chai';
88
import {afterEach, beforeEach} from 'mocha';
99
import sinon from 'sinon';
1010
import CodeActivate from '../../../src/commands/code/activate.js';
11-
import {createIsolatedConfigHooks, createTestCommand} from '../../helpers/test-setup.js';
11+
import {createIsolatedConfigHooks, createTestCommand, expectError} from '../../helpers/test-setup.js';
1212

1313
describe('code activate', () => {
1414
const hooks = createIsolatedConfigHooks();
@@ -39,8 +39,11 @@ describe('code activate', () => {
3939

4040
await command.run();
4141

42-
expect(patchStub.calledOnce).to.equal(true);
43-
expect(patchStub.getCall(0).args[0]).to.equal('/code_versions/{code_version_id}');
42+
expect(patchStub.calledOnce).to.be.true;
43+
const [path, options] = patchStub.firstCall.args;
44+
expect(path).to.equal('/code_versions/{code_version_id}');
45+
expect(options?.params?.path).to.deep.equal({code_version_id: 'v1'});
46+
expect(options?.body).to.deep.equal({active: true});
4447
});
4548

4649
it('errors when no code version is provided for activate mode', async () => {
@@ -51,14 +54,10 @@ describe('code activate', () => {
5154

5255
const errorStub = sinon.stub(command, 'error').throws(new Error('Expected error'));
5356

54-
try {
55-
await command.run();
56-
expect.fail('Should have thrown');
57-
} catch {
58-
// expected
59-
}
57+
await expectError(() => command.run());
6058

61-
expect(errorStub.calledOnce).to.equal(true);
59+
expect(errorStub.calledOnce).to.be.true;
60+
expect(errorStub.firstCall.args[0]).to.match(/code version/i);
6261
});
6362

6463
it('reloads the active code version when --reload is set and no arg is provided', async () => {
@@ -90,8 +89,11 @@ describe('code activate', () => {
9089

9190
await command.run();
9291

93-
expect(getStub.calledOnce).to.equal(true);
92+
expect(getStub.calledOnce).to.be.true;
9493
expect(patchStub.callCount).to.equal(2);
94+
// Reload toggles to alternate then back to active.
95+
const calledIds = patchStub.getCalls().map((c) => c.args[1]?.params?.path?.code_version_id);
96+
expect(calledIds).to.deep.equal(['v2', 'v1']);
9597
});
9698

9799
it('calls command.error when reload fails with an error message', async () => {
@@ -102,9 +104,13 @@ describe('code activate', () => {
102104

103105
sinon.stub(command, 'resolvedConfig').get(() => ({values: {hostname: 'example.com', codeVersion: undefined}}));
104106

107+
// Reload toggles active → alternate → active, so we need at least two versions.
105108
const getStub = sinon.stub().resolves({
106109
data: {
107-
data: [{id: 'v1', active: true}],
110+
data: [
111+
{id: 'v1', active: true},
112+
{id: 'v2', active: false},
113+
],
108114
},
109115
error: undefined,
110116
});
@@ -120,13 +126,10 @@ describe('code activate', () => {
120126

121127
const errorStub = sinon.stub(command, 'error').throws(new Error('Expected error'));
122128

123-
try {
124-
await command.run();
125-
expect.fail('Should have thrown');
126-
} catch {
127-
// expected
128-
}
129+
await expectError(() => command.run());
129130

130-
expect(errorStub.calledOnce).to.equal(true);
131+
expect(errorStub.calledOnce).to.be.true;
132+
expect(errorStub.firstCall.args[0]).to.include('Failed to reload code version');
133+
expect(patchStub.called).to.be.true;
131134
});
132135
});

packages/b2c-cli/test/commands/code/deploy.test.ts

Lines changed: 23 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {expect} from 'chai';
88
import {afterEach, beforeEach} from 'mocha';
99
import sinon from 'sinon';
1010
import CodeDeploy from '../../../src/commands/code/deploy.js';
11-
import {createIsolatedConfigHooks, createTestCommand} from '../../helpers/test-setup.js';
11+
import {createIsolatedConfigHooks, createTestCommand, expectError} from '../../helpers/test-setup.js';
1212

1313
describe('code deploy', () => {
1414
const hooks = createIsolatedConfigHooks();
@@ -54,14 +54,10 @@ describe('code deploy', () => {
5454

5555
const errorStub = sinon.stub(command, 'error').throws(new Error('Expected error'));
5656

57-
try {
58-
await command.run();
59-
expect.fail('Should have thrown');
60-
} catch {
61-
// expected
62-
}
57+
await expectError(() => command.run());
6358

64-
expect(errorStub.calledOnce).to.equal(true);
59+
expect(errorStub.calledOnce).to.be.true;
60+
expect(errorStub.firstCall.args[0]).to.include('No cartridges found');
6561
});
6662

6763
it('calls delete + upload and reload when flags are set', async () => {
@@ -86,14 +82,15 @@ describe('code deploy', () => {
8682

8783
const result = await command.run();
8884

89-
expect(deleteStub.calledOnceWithExactly(instance, cartridges)).to.equal(true);
90-
expect(uploadStub.calledOnce).to.equal(true);
85+
expect(deleteStub.calledOnceWithExactly(instance, cartridges)).to.be.true;
86+
expect(uploadStub.calledOnce).to.be.true;
9187
expect(uploadStub.firstCall.args[0]).to.equal(instance);
9288
expect(uploadStub.firstCall.args[1]).to.equal(cartridges);
93-
expect(reloadStub.calledOnceWithExactly(instance, 'v1')).to.equal(true);
89+
expect(reloadStub.calledOnceWithExactly(instance, 'v1')).to.be.true;
9490

9591
expect(result).to.deep.include({codeVersion: 'v1', activated: true, reloaded: true});
96-
expect(afterHooksStub.calledOnce).to.equal(true);
92+
expect(afterHooksStub.calledOnce).to.be.true;
93+
expect(afterHooksStub.firstCall.args[1]).to.deep.include({success: true});
9794
});
9895

9996
it('calls activate after deploy when --activate is set', async () => {
@@ -112,7 +109,10 @@ describe('code deploy', () => {
112109

113110
const result = await command.run();
114111

115-
expect(activateStub.calledOnceWithExactly(instance, 'v1')).to.equal(true);
112+
expect(activateStub.calledOnceWithExactly(instance, 'v1')).to.be.true;
113+
expect(uploadStub.calledOnce).to.be.true;
114+
expect(uploadStub.firstCall.args[0]).to.equal(instance);
115+
expect(uploadStub.firstCall.args[1]).to.equal(cartridges);
116116
expect(result).to.deep.include({codeVersion: 'v1', activated: true, reloaded: false});
117117
});
118118

@@ -132,14 +132,9 @@ describe('code deploy', () => {
132132

133133
const errorStub = sinon.stub(command, 'error').throws(new Error('Expected error'));
134134

135-
try {
136-
await command.run();
137-
expect.fail('Should have thrown');
138-
} catch {
139-
// expected
140-
}
135+
await expectError(() => command.run());
141136

142-
expect(errorStub.called).to.equal(true);
137+
expect(errorStub.called).to.be.true;
143138
const errorMessage = errorStub.firstCall.args[0];
144139
expect(errorMessage).to.include('activate failed');
145140
expect(errorMessage).to.include('OCAPI');
@@ -161,14 +156,9 @@ describe('code deploy', () => {
161156

162157
const errorStub = sinon.stub(command, 'error').throws(new Error('Expected error'));
163158

164-
try {
165-
await command.run();
166-
expect.fail('Should have thrown');
167-
} catch {
168-
// expected
169-
}
159+
await expectError(() => command.run());
170160

171-
expect(errorStub.called).to.equal(true);
161+
expect(errorStub.called).to.be.true;
172162
const errorMessage = errorStub.firstCall.args[0];
173163
expect(errorMessage).to.include('reload failed');
174164
expect(errorMessage).to.include('OCAPI');
@@ -185,14 +175,9 @@ describe('code deploy', () => {
185175

186176
const errorStub = sinon.stub(command, 'error').throws(new Error('OAuth required'));
187177

188-
try {
189-
await command.run();
190-
expect.fail('Should have thrown');
191-
} catch {
192-
// expected
193-
}
178+
await expectError(() => command.run());
194179

195-
expect(errorStub.calledOnce).to.equal(true);
180+
expect(errorStub.calledOnce).to.be.true;
196181
const errorMessage = errorStub.firstCall.args[0];
197182
expect(errorMessage).to.include('auto-discover');
198183
});
@@ -208,14 +193,9 @@ describe('code deploy', () => {
208193

209194
const errorStub = sinon.stub(command, 'error').throws(new Error('OAuth required'));
210195

211-
try {
212-
await command.run();
213-
expect.fail('Should have thrown');
214-
} catch {
215-
// expected
216-
}
196+
await expectError(() => command.run());
217197

218-
expect(errorStub.calledOnce).to.equal(true);
198+
expect(errorStub.calledOnce).to.be.true;
219199
const errorMessage = errorStub.firstCall.args[0];
220200
expect(errorMessage).to.include('activate');
221201
});
@@ -231,14 +211,9 @@ describe('code deploy', () => {
231211

232212
const errorStub = sinon.stub(command, 'error').throws(new Error('OAuth required'));
233213

234-
try {
235-
await command.run();
236-
expect.fail('Should have thrown');
237-
} catch {
238-
// expected
239-
}
214+
await expectError(() => command.run());
240215

241-
expect(errorStub.calledOnce).to.equal(true);
216+
expect(errorStub.calledOnce).to.be.true;
242217
const errorMessage = errorStub.firstCall.args[0];
243218
expect(errorMessage).to.include('activate');
244219
});

packages/b2c-cli/test/commands/mrt/env/var/push.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ describe('mrt env var push', () => {
125125

126126
await command.run();
127127

128+
expect(listStub.calledOnce, 'remote env vars must be fetched to compute diff').to.be.true;
128129
expect(setBatchStub.called).to.be.false;
129130
expect(setStub.called).to.be.false;
130131
});

packages/b2c-cli/test/commands/sandbox/create.test.ts

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {expect} from 'chai';
88
import sinon from 'sinon';
99
import SandboxCreate from '../../../src/commands/sandbox/create.js';
1010
import {isolateConfig, restoreConfig} from '@salesforce/b2c-tooling-sdk/test-utils';
11-
import {runSilent} from '../../helpers/test-setup.js';
11+
import {makeCommandThrowOnError, runSilent, stubOdsClient} from '../../helpers/test-setup.js';
1212

1313
function stubCommandConfigAndLogger(command: any, sandboxApiHost = 'admin.dx.test.com'): void {
1414
Object.defineProperty(command, 'config', {
@@ -26,26 +26,13 @@ function stubCommandConfigAndLogger(command: any, sandboxApiHost = 'admin.dx.tes
2626
});
2727
}
2828

29-
function stubOdsClient(command: any, client: Partial<{GET: any; POST: any; PUT: any; DELETE: any}>): void {
30-
Object.defineProperty(command, 'odsClient', {
31-
value: client,
32-
configurable: true,
33-
});
34-
}
35-
3629
function stubResolvedConfig(command: any, resolvedConfig: Record<string, unknown>): void {
3730
Object.defineProperty(command, 'resolvedConfig', {
3831
get: () => ({values: resolvedConfig}),
3932
configurable: true,
4033
});
4134
}
4235

43-
function makeCommandThrowOnError(command: any): void {
44-
command.error = (msg: string) => {
45-
throw new Error(msg);
46-
};
47-
}
48-
4936
/**
5037
* Unit tests for ODS create command CLI logic.
5138
* Tests settings building, permission logic, wait/poll logic.

0 commit comments

Comments
 (0)