Skip to content

Commit ef77802

Browse files
committed
refactor: improve test quality and code style
- Replace try/catch/fail pattern with await expect().rejects.toThrow() - Replace lazy .toContain() with precise .toBe() for error messages - Fix bracket notation: options['user'] → options.user - Remove obsolete historical documentation about v3 Promise bug
1 parent 6e07397 commit ef77802

File tree

3 files changed

+20
-86
lines changed

3 files changed

+20
-86
lines changed

src/deploy/actions.spec.ts

Lines changed: 19 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,10 @@ describe('Deploy Angular apps', () => {
6565
it('throws if there is no target project', async () => {
6666
context.target = undefined;
6767
const expectedErrorMessage = 'Cannot execute the build target';
68-
try {
69-
await deploy(mockEngine, context, BUILD_TARGET, {});
70-
fail();
71-
} catch (e) {
72-
expect(e.message).toBe(expectedErrorMessage);
73-
}
68+
69+
await expect(
70+
deploy(mockEngine, context, BUILD_TARGET, {})
71+
).rejects.toThrow(expectedErrorMessage);
7472
});
7573

7674
it('throws if app building fails', async () => {
@@ -82,12 +80,10 @@ describe('Deploy Angular apps', () => {
8280
Promise.resolve({
8381
result: Promise.resolve(createBuilderOutputMock(false))
8482
} as BuilderRun);
85-
try {
86-
await deploy(mockEngine, context, BUILD_TARGET, {});
87-
fail();
88-
} catch (e) {
89-
expect(e.message).toEqual('Error while building the app.');
90-
}
83+
84+
await expect(
85+
deploy(mockEngine, context, BUILD_TARGET, {})
86+
).rejects.toThrow('Error while building the app.');
9187
});
9288

9389
it('throws if outputPath is missing from build options', async () => {
@@ -96,12 +92,9 @@ describe('Deploy Angular apps', () => {
9692

9793
const expectedErrorMessage = `Cannot read the outputPath option of the Angular project '${BUILD_TARGET.name}' in angular.json.`;
9894

99-
try {
100-
await deploy(mockEngine, context, BUILD_TARGET, { noBuild: false });
101-
fail();
102-
} catch (e) {
103-
expect(e.message).toBe(expectedErrorMessage);
104-
}
95+
await expect(
96+
deploy(mockEngine, context, BUILD_TARGET, { noBuild: false })
97+
).rejects.toThrow(expectedErrorMessage);
10598
});
10699

107100
it('throws if outputPath has invalid shape (not string or object)', async () => {
@@ -110,14 +103,11 @@ describe('Deploy Angular apps', () => {
110103
outputPath: 123
111104
} as JsonObject);
112105

113-
const expectedErrorPrefix = 'Unsupported outputPath configuration';
106+
const expectedErrorMessage = `Unsupported outputPath configuration in angular.json for '${BUILD_TARGET.name}'. Expected string or {base, browser} object.`;
114107

115-
try {
116-
await deploy(mockEngine, context, BUILD_TARGET, { noBuild: false });
117-
fail();
118-
} catch (e) {
119-
expect(e.message).toContain(expectedErrorPrefix);
120-
}
108+
await expect(
109+
deploy(mockEngine, context, BUILD_TARGET, { noBuild: false })
110+
).rejects.toThrow(expectedErrorMessage);
121111
});
122112

123113
it('throws if outputPath object is missing base property', async () => {
@@ -126,14 +116,11 @@ describe('Deploy Angular apps', () => {
126116
outputPath: { browser: 'browser' }
127117
} as JsonObject);
128118

129-
const expectedErrorPrefix = 'Unsupported outputPath configuration';
119+
const expectedErrorMessage = `Unsupported outputPath configuration in angular.json for '${BUILD_TARGET.name}'. Expected string or {base, browser} object.`;
130120

131-
try {
132-
await deploy(mockEngine, context, BUILD_TARGET, { noBuild: false });
133-
fail();
134-
} catch (e) {
135-
expect(e.message).toContain(expectedErrorPrefix);
136-
}
121+
await expect(
122+
deploy(mockEngine, context, BUILD_TARGET, { noBuild: false })
123+
).rejects.toThrow(expectedErrorMessage);
137124
});
138125

139126
it('uses correct dir when outputPath is object with base and browser (OP1)', async () => {

src/engine/engine.gh-pages-behavior.spec.ts

Lines changed: 0 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -758,17 +758,11 @@ describe('gh-pages v6.3.0 - behavioral snapshot', () => {
758758
* 1. Git clone failure and retry (branch doesn't exist)
759759
* 2. No changes to commit (diff-index returns 0)
760760
* 3. Git commit when no user configured (uses getUser)
761-
* 4. Early error handling (invalid basePath) - PR #186 compatibility
762761
*
763762
* Why test error scenarios:
764763
* - Ensures deployments don't fail silently
765764
* - Verifies retry/fallback mechanisms work
766765
* - Documents expected error recovery behavior
767-
*
768-
* Historical note:
769-
* gh-pages v3.1.0 had a bug where early errors didn't return promises.
770-
* gh-pages v5.0.0+ fixed this to correctly return Promise.reject().
771-
* We now use the Promise-based API directly (await ghPages.publish()).
772766
*/
773767
describe('Error scenarios', () => {
774768
it('should retry git clone without branch/depth options on failure', (done) => {
@@ -999,53 +993,6 @@ describe('gh-pages v6.3.0 - behavioral snapshot', () => {
999993
});
1000994
});
1001995

1002-
/**
1003-
* PR #186 Issue #1: gh-pages Promise Bug (documented here)
1004-
*
1005-
* Context from PR #186 analysis:
1006-
* gh-pages v3.1.0 has a bug where early error cases don't return a promise.
1007-
* Source: https://github.com/tschaub/gh-pages/blob/v3.1.0/lib/index.js#L78-85
1008-
*
1009-
* Evidence from source code:
1010-
* ```
1011-
* try {
1012-
* if (!fs.statSync(basePath).isDirectory()) {
1013-
* done(new Error('The "base" option must be an existing directory'));
1014-
* return; // ❌ NO PROMISE RETURNED!
1015-
* }
1016-
* } catch (err) {
1017-
* done(err);
1018-
* return; // ❌ NO PROMISE RETURNED!
1019-
* }
1020-
* ```
1021-
*
1022-
* Early errors include:
1023-
* - basePath is not a directory
1024-
* - basePath doesn't exist
1025-
* - No files match src pattern
1026-
*
1027-
* In gh-pages v3.x, if you used await ghPages.publish(), these errors were silently swallowed!
1028-
*
1029-
* HISTORICAL: Our old workaround (REMOVED in v6 upgrade):
1030-
* ```
1031-
* // do NOT (!!) await ghPages.publish,
1032-
* // the promise is implemented in such a way that it always succeeds – even on errors!
1033-
* return new Promise((resolve, reject) => {
1034-
* ghPages.publish(dir, options, error => {
1035-
* if (error) { return reject(error); }
1036-
* resolve(undefined);
1037-
* });
1038-
* });
1039-
* ```
1040-
*
1041-
* CURRENT (gh-pages v6.3.0):
1042-
* - Fixed in gh-pages v5.0.0+: All errors correctly return Promise.reject()
1043-
* - We now use `await ghPages.publish()` directly (engine.ts:203-204)
1044-
* - The Promise bug is fixed, all error cases properly reject
1045-
* - This documentation preserved for historical context only
1046-
*
1047-
* Current behavior is tested in engine.spec.ts integration tests.
1048-
*/
1049996
});
1050997

1051998
/**

src/engine/engine.prepare-options-helpers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ export function handleUserCredentials(
104104
logger: logging.LoggerApi
105105
): void {
106106
if (options.name && options.email) {
107-
options['user'] = {
107+
options.user = {
108108
name: options.name,
109109
email: options.email
110110
};

0 commit comments

Comments
 (0)