Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 38 additions & 8 deletions src/engine/engine-filesystem.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,12 @@ describe('engine - real filesystem tests', () => {

const ghpages = require('gh-pages');
jest.spyOn(ghpages, 'clean').mockImplementation(() => {});
jest.spyOn(ghpages, 'publish').mockResolvedValue(undefined);
jest.spyOn(ghpages, 'publish').mockImplementation((_dir: unknown, _opts: unknown, callback?: (error: Error | null) => void) => {
if (callback) {
callback(null);
}
return Promise.resolve(undefined);
});

const options = {
notfound: true,
Expand All @@ -84,7 +89,12 @@ describe('engine - real filesystem tests', () => {

const ghpages = require('gh-pages');
jest.spyOn(ghpages, 'clean').mockImplementation(() => {});
jest.spyOn(ghpages, 'publish').mockResolvedValue(undefined);
jest.spyOn(ghpages, 'publish').mockImplementation((_dir: unknown, _opts: unknown, callback?: (error: Error | null) => void) => {
if (callback) {
callback(null);
}
return Promise.resolve(undefined);
});

const options = {
notfound: false,
Expand All @@ -104,7 +114,12 @@ describe('engine - real filesystem tests', () => {

const ghpages = require('gh-pages');
jest.spyOn(ghpages, 'clean').mockImplementation(() => {});
jest.spyOn(ghpages, 'publish').mockResolvedValue(undefined);
jest.spyOn(ghpages, 'publish').mockImplementation((_dir: unknown, _opts: unknown, callback?: (error: Error | null) => void) => {
if (callback) {
callback(null);
}
return Promise.resolve(undefined);
});

const options = {
notfound: true,
Expand Down Expand Up @@ -173,8 +188,11 @@ describe('engine - real filesystem tests', () => {

let capturedOptions: { cname?: string; nojekyll?: boolean } = {};
const publishSpy = jest.spyOn(ghpages, 'publish').mockImplementation(
(dir: string, options: { cname?: string; nojekyll?: boolean }) => {
(_dir: string, options: { cname?: string; nojekyll?: boolean }, callback?: (error: Error | null) => void) => {
capturedOptions = options;
if (callback) {
callback(null);
}
return Promise.resolve();
}
);
Expand Down Expand Up @@ -202,8 +220,11 @@ describe('engine - real filesystem tests', () => {

let capturedOptions: { cname?: string; nojekyll?: boolean } = {};
const publishSpy = jest.spyOn(ghpages, 'publish').mockImplementation(
(dir: string, options: { cname?: string; nojekyll?: boolean }) => {
(_dir: string, options: { cname?: string; nojekyll?: boolean }, callback?: (error: Error | null) => void) => {
capturedOptions = options;
if (callback) {
callback(null);
}
return Promise.resolve();
}
);
Expand All @@ -229,8 +250,11 @@ describe('engine - real filesystem tests', () => {

let capturedOptions: { cname?: string; nojekyll?: boolean } = {};
const publishSpy = jest.spyOn(ghpages, 'publish').mockImplementation(
(dir: string, options: { cname?: string; nojekyll?: boolean }) => {
(_dir: string, options: { cname?: string; nojekyll?: boolean }, callback?: (error: Error | null) => void) => {
capturedOptions = options;
if (callback) {
callback(null);
}
return Promise.resolve();
}
);
Expand Down Expand Up @@ -263,8 +287,11 @@ describe('engine - real filesystem tests', () => {

let capturedOptions: { cname?: string; nojekyll?: boolean } = {};
const publishSpy = jest.spyOn(ghpages, 'publish').mockImplementation(
(dir: string, options: { cname?: string; nojekyll?: boolean }) => {
(_dir: string, options: { cname?: string; nojekyll?: boolean }, callback?: (error: Error | null) => void) => {
capturedOptions = options;
if (callback) {
callback(null);
}
return Promise.resolve();
}
);
Expand All @@ -291,8 +318,11 @@ describe('engine - real filesystem tests', () => {

let capturedOptions: { cname?: string; nojekyll?: boolean } = {};
const publishSpy = jest.spyOn(ghpages, 'publish').mockImplementation(
(dir: string, options: { cname?: string; nojekyll?: boolean }) => {
(_dir: string, options: { cname?: string; nojekyll?: boolean }, callback?: (error: Error | null) => void) => {
capturedOptions = options;
if (callback) {
callback(null);
}
return Promise.resolve();
}
);
Expand Down
50 changes: 50 additions & 0 deletions src/engine/engine.gh-pages-behavior.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1016,4 +1016,54 @@ describe('gh-pages v6.3.0 - behavioral snapshot', () => {
* gh-pages.clean() behavior is tested in engine.gh-pages-clean.spec.ts
* That test uses real filesystem operations without mocks.
*/

/**
* End-to-end canary for issue #205: drives the REAL gh-pages library
* through engine.run() with a mocked child_process.spawn that simulates
* an HTTPS auth failure during `git clone`. If upstream gh-pages ever
* fixes the `.then(_, onRejected)` absorption pattern, this test will
* still pass; if they change callback semantics, it will fail loudly.
*/
describe('auth-failure silent-swallow regression (issue #205)', () => {
const engine = require('./engine');
const { cleanupMonkeypatch } = require('./engine.prepare-options-helpers');
const { logging } = require('@angular-devkit/core');

beforeEach(() => {
cleanupMonkeypatch();
});

it('engine.run() should reject when git clone fails with fatal: Authentication failed', async () => {
mockSpawn.mockImplementationOnce((cmd: string, args: string[] | undefined) => {
const capturedArgs = args || [];
spawnCalls.push({ cmd, args: capturedArgs, options: undefined });
const child = createMockChildProcess();
setImmediate(() => {
child.stderr!.emit(
'data',
Buffer.from(
"fatal: Authentication failed for 'https://github.com/owner/repo.git'"
)
);
child.emit!('close', 128);
});
return child;
});

const options = {
repo: 'https://x-access-token:bad-token@github.com/owner/repo.git',
branch: 'gh-pages',
dotfiles: true,
notfound: true,
nojekyll: true
};

await expect(
engine.run(basePath, options, new logging.NullLogger())
).rejects.toThrow();

expect(spawnCalls[0]?.cmd).toBe('git');
expect(spawnCalls[0]?.args[0]).toBe('clone');
});
});
});
93 changes: 81 additions & 12 deletions src/engine/engine.gh-pages-integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,14 @@ describe('engine - gh-pages integration', () => {
ghpagesPublishSpy = jest.spyOn(ghpages, 'publish');
}

// Set default mock implementations - gh-pages v5+ uses Promise-based API
// engine uses the callback form of gh-pages.publish() — see #205
ghpagesCleanSpy.mockImplementation(() => {});
ghpagesPublishSpy.mockResolvedValue(undefined);
ghpagesPublishSpy.mockImplementation((_dir: string, _opts: unknown, callback?: (error: Error | null) => void) => {
if (callback) {
callback(null);
}
return Promise.resolve(undefined);
});

// Create fresh copy of environment for each test
// This preserves PATH, HOME, etc. needed by git
Expand Down Expand Up @@ -124,10 +129,11 @@ describe('engine - gh-pages integration', () => {
await engine.run(testDir, options, logger);

expect(ghpagesPublishSpy).toHaveBeenCalledTimes(1);
// gh-pages v5+ uses Promise-based API (no callback)
// engine uses the callback form of gh-pages.publish() — see #205
expect(ghpagesPublishSpy).toHaveBeenCalledWith(
testDir,
expect.any(Object)
expect.any(Object),
expect.any(Function)
);
});

Expand Down Expand Up @@ -515,23 +521,28 @@ describe('engine - gh-pages integration', () => {
});

describe('Promise handling integration', () => {
// gh-pages v5+ uses Promise-based API (we no longer use callback-based approach)
// engine uses the callback form of gh-pages.publish() — see #205

it('should invoke gh-pages.publish() without callback (Promise-based)', async () => {
it('should invoke gh-pages.publish() with a callback', async () => {
const testDir = '/test/dist';
const options = { dotfiles: true, notfound: true, nojekyll: true };

await engine.run(testDir, options, logger);

// gh-pages v5+ Promise API: publish(dir, options) - no callback
expect(ghpagesPublishSpy).toHaveBeenCalledWith(
expect.any(String),
expect.any(Object)
expect.any(Object),
expect.any(Function)
);
});

it('should resolve when gh-pages.publish() resolves', async () => {
ghpagesPublishSpy.mockResolvedValue(undefined);
it('should resolve when gh-pages.publish() invokes the callback with no error', async () => {
ghpagesPublishSpy.mockImplementation((_dir: string, _opts: unknown, callback?: (error: Error | null) => void) => {
if (callback) {
callback(null);
}
return Promise.resolve(undefined);
});

const testDir = '/test/dist';
const options = { dotfiles: true, notfound: true, nojekyll: true };
Expand All @@ -541,9 +552,14 @@ describe('engine - gh-pages integration', () => {
).resolves.toBeUndefined();
});

it('should reject when gh-pages.publish() rejects', async () => {
it('should reject when gh-pages.publish() invokes the callback with an error', async () => {
const publishError = new Error('Git push failed');
ghpagesPublishSpy.mockRejectedValue(publishError);
ghpagesPublishSpy.mockImplementation((_dir: string, _opts: unknown, callback?: (error: Error | null) => void) => {
if (callback) {
callback(publishError);
}
return Promise.resolve(undefined);
});

const testDir = '/test/dist';
const options = { dotfiles: true, notfound: true, nojekyll: true };
Expand All @@ -553,4 +569,57 @@ describe('engine - gh-pages integration', () => {
).rejects.toThrow('Git push failed');
});
});

describe('silent-swallow regression (issue #205)', () => {
// gh-pages@6 absorbs errors into its returned Promise via an internal
// .then(_, onRejected) handler that doesn't rethrow. The callback, however,
// still fires with the error. Engine must use the callback form so git
// failures (e.g. auth errors during clone) surface as rejections.

it('should reject when gh-pages resolves its promise but delivers an error via callback', async () => {
const authError = new Error(
"fatal: Authentication failed for 'https://github.com/owner/repo.git'"
);

ghpagesPublishSpy.mockImplementation((_dir: string, _opts: unknown, callback?: (error: Error | null) => void) => {
if (callback) {
callback(authError);
}
return Promise.resolve(undefined);
});

const testDir = '/test/dist';
const options = { dotfiles: true, notfound: true, nojekyll: true };

await expect(
engine.run(testDir, options, logger)
).rejects.toThrow(/Authentication failed/);
});

it('should NOT log the success banner when publish fails via callback', async () => {
const authError = new Error('fatal: Authentication failed');

ghpagesPublishSpy.mockImplementation((_dir: string, _opts: unknown, callback?: (error: Error | null) => void) => {
if (callback) {
callback(authError);
}
return Promise.resolve(undefined);
});

const testLogger = new logging.Logger('test');
const infoSpy = jest.spyOn(testLogger, 'info');

const testDir = '/test/dist';
const options = { dotfiles: true, notfound: true, nojekyll: true };

await expect(
engine.run(testDir, options, testLogger)
).rejects.toThrow();

const bannerCall = infoSpy.mock.calls.find(
([msg]) => typeof msg === 'string' && msg.includes('Successfully published')
);
expect(bannerCall).toBeUndefined();
});
});
});
19 changes: 13 additions & 6 deletions src/engine/engine.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,11 +279,19 @@ describe('engine', () => {
ghpagesPublishSpy.mockRestore();
});

// engine uses the callback form of gh-pages.publish() — see #205
const mockPublishCallback = (error: Error | null) =>
(_dir: unknown, _opts: unknown, callback?: (err: Error | null) => void) => {
if (callback) {
callback(error);
}
return Promise.resolve(undefined);
};

it('should reject when gh-pages.publish rejects with error', async () => {
const publishError = new Error('Git push failed: permission denied');

// gh-pages v5+ properly rejects with errors
ghpagesPublishSpy.mockRejectedValue(publishError);
ghpagesPublishSpy.mockImplementation(mockPublishCallback(publishError));

const testDir = '/test/dist';
const options = { dotfiles: true, notfound: true, nojekyll: true };
Expand All @@ -296,7 +304,7 @@ describe('engine', () => {
it('should preserve error message through rejection', async () => {
const detailedError = new Error('Remote url mismatch. Expected https://github.com/user/repo.git but got https://github.com/other/repo.git');

ghpagesPublishSpy.mockRejectedValue(detailedError);
ghpagesPublishSpy.mockImplementation(mockPublishCallback(detailedError));

const testDir = '/test/dist';
const options = { dotfiles: true, notfound: true, nojekyll: true };
Expand All @@ -309,7 +317,7 @@ describe('engine', () => {
it('should reject with authentication error from gh-pages', async () => {
const authError = new Error('Authentication failed: Invalid credentials');

ghpagesPublishSpy.mockRejectedValue(authError);
ghpagesPublishSpy.mockImplementation(mockPublishCallback(authError));

const testDir = '/test/dist';
const options = { dotfiles: true, notfound: true, nojekyll: true };
Expand All @@ -320,8 +328,7 @@ describe('engine', () => {
});

it('should resolve successfully when gh-pages.publish resolves', async () => {
// gh-pages v5+ resolves the Promise on success
ghpagesPublishSpy.mockResolvedValue(undefined);
ghpagesPublishSpy.mockImplementation(mockPublishCallback(null));

const testDir = '/test/dist';
const options = { dotfiles: true, notfound: true, nojekyll: true };
Expand Down
16 changes: 13 additions & 3 deletions src/engine/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,17 @@ async function publishViaGhPages(
nojekyll: options.nojekyll
};

// gh-pages v5+ fixed the Promise bug where errors didn't reject properly
// We can now safely await the promise directly
await ghPages.publish(dir, ghPagesOptions);
// gh-pages@6 silently absorbs errors via its internal .then(_, onRejected)
// handler (and returns undefined from some early-exit paths, upstream #465).
// The callback always fires with the error, so we bridge that to a rejection.
// See issue #205.
await new Promise<void>((resolve, reject) => {
ghPages.publish(dir, ghPagesOptions, (error: Error | null) => {
if (error) {
reject(error);
return;
}
resolve();
});
});
}
4 changes: 2 additions & 2 deletions src/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "angular-cli-ghpages",
"version": "3.0.2",
"version": "3.0.3",
"description": "Deploy your Angular app to GitHub Pages or Cloudflare Pages directly from the Angular CLI (ng deploy)",
"main": "index.js",
"types": "index.d.ts",
Expand Down
Loading