diff --git a/src/engine/engine-filesystem.spec.ts b/src/engine/engine-filesystem.spec.ts index 2d5e254..b24c081 100644 --- a/src/engine/engine-filesystem.spec.ts +++ b/src/engine/engine-filesystem.spec.ts @@ -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, @@ -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, @@ -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, @@ -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(); } ); @@ -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(); } ); @@ -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(); } ); @@ -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(); } ); @@ -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(); } ); diff --git a/src/engine/engine.gh-pages-behavior.spec.ts b/src/engine/engine.gh-pages-behavior.spec.ts index 73d9fc5..67fcdbf 100644 --- a/src/engine/engine.gh-pages-behavior.spec.ts +++ b/src/engine/engine.gh-pages-behavior.spec.ts @@ -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'); + }); + }); }); diff --git a/src/engine/engine.gh-pages-integration.spec.ts b/src/engine/engine.gh-pages-integration.spec.ts index d7e2090..b1c09cf 100644 --- a/src/engine/engine.gh-pages-integration.spec.ts +++ b/src/engine/engine.gh-pages-integration.spec.ts @@ -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 @@ -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) ); }); @@ -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 }; @@ -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 }; @@ -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(); + }); + }); }); diff --git a/src/engine/engine.spec.ts b/src/engine/engine.spec.ts index a344a84..f73e0ee 100644 --- a/src/engine/engine.spec.ts +++ b/src/engine/engine.spec.ts @@ -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 }; @@ -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 }; @@ -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 }; @@ -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 }; diff --git a/src/engine/engine.ts b/src/engine/engine.ts index 6a8d1d9..39766da 100644 --- a/src/engine/engine.ts +++ b/src/engine/engine.ts @@ -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((resolve, reject) => { + ghPages.publish(dir, ghPagesOptions, (error: Error | null) => { + if (error) { + reject(error); + return; + } + resolve(); + }); + }); } diff --git a/src/package-lock.json b/src/package-lock.json index 4703c9c..cdb2895 100644 --- a/src/package-lock.json +++ b/src/package-lock.json @@ -1,12 +1,12 @@ { "name": "angular-cli-ghpages", - "version": "3.0.2", + "version": "3.0.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "angular-cli-ghpages", - "version": "3.0.2", + "version": "3.0.3", "license": "MIT", "dependencies": { "@angular-devkit/architect": ">=0.1800.0 <0.2200.0", diff --git a/src/package.json b/src/package.json index ec3f386..759a74b 100644 --- a/src/package.json +++ b/src/package.json @@ -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", diff --git a/src/parameter-tests/builder-integration.spec.ts b/src/parameter-tests/builder-integration.spec.ts index efc0240..b147f1a 100644 --- a/src/parameter-tests/builder-integration.spec.ts +++ b/src/parameter-tests/builder-integration.spec.ts @@ -34,11 +34,14 @@ jest.mock('gh-pages/lib/git', () => { })); }); -// Mock gh-pages module - gh-pages v5+ uses Promise-based API +// Mock gh-pages module — engine uses the callback form (#205) jest.mock('gh-pages', () => ({ clean: jest.fn(), - publish: jest.fn((dir: string, options: PublishOptions) => { + publish: jest.fn((_dir: string, options: PublishOptions, callback?: (error: Error | null) => void) => { capturedPublishOptions = options; + if (callback) { + callback(null); + } return Promise.resolve(); }) }));