Skip to content

Commit 2e024fe

Browse files
JohannesHoppeJohannesHoppe
authored
refactor: remove fs-extra, use native fs/promises (#202)
- Replace fs-extra with Node.js native fs/promises module - Add pathExists utility function to utils.ts - Add tests for pathExists and createHost utilities - Fix flaky gh-pages tests by using push:false instead of aborting mid-operation - Remove test that called ghPages.clean() globally (caused race conditions) - Bump version to 3.0.2
1 parent 512cec8 commit 2e024fe

14 files changed

+325
-271
lines changed

‎src/engine/engine-filesystem.spec.ts‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,13 @@
1111
*/
1212

1313
import { logging } from '@angular-devkit/core';
14-
import * as fse from 'fs-extra';
14+
import * as fs from 'fs/promises';
1515
import * as os from 'os';
1616
import * as path from 'path';
1717

1818
import * as engine from './engine';
1919
import { cleanupMonkeypatch } from './engine.prepare-options-helpers';
20+
import { pathExists } from '../utils';
2021

2122
describe('engine - real filesystem tests', () => {
2223
const logger = new logging.Logger('test');
@@ -31,16 +32,16 @@ describe('engine - real filesystem tests', () => {
3132
const tmpBase = os.tmpdir();
3233
const uniqueDir = `angular-cli-ghpages-test-${Date.now()}-${Math.random().toString(36).substring(7)}`;
3334
testDir = path.join(tmpBase, uniqueDir);
34-
await fse.ensureDir(testDir);
35+
await fs.mkdir(testDir, { recursive: true });
3536

3637
// Spy on logger to capture warnings
3738
loggerInfoSpy = jest.spyOn(logger, 'info');
3839
});
3940

4041
afterEach(async () => {
4142
// Clean up temp directory after each test
42-
if (await fse.pathExists(testDir)) {
43-
await fse.remove(testDir);
43+
if (await pathExists(testDir)) {
44+
await fs.rm(testDir, { recursive: true });
4445
}
4546
loggerInfoSpy.mockRestore();
4647
});
@@ -55,7 +56,7 @@ describe('engine - real filesystem tests', () => {
5556
// First create an index.html file
5657
const indexPath = path.join(testDir, 'index.html');
5758
const indexContent = '<!DOCTYPE html><html><head><title>Test</title></head><body><h1>Test App</h1></body></html>';
58-
await fse.writeFile(indexPath, indexContent);
59+
await fs.writeFile(indexPath, indexContent);
5960

6061
const ghpages = require('gh-pages');
6162
jest.spyOn(ghpages, 'clean').mockImplementation(() => {});
@@ -70,16 +71,16 @@ describe('engine - real filesystem tests', () => {
7071
await engine.run(testDir, options, logger);
7172

7273
const notFoundPath = path.join(testDir, '404.html');
73-
const exists = await fse.pathExists(notFoundPath);
74+
const exists = await pathExists(notFoundPath);
7475
expect(exists).toBe(true);
7576

76-
const notFoundContent = await fse.readFile(notFoundPath, 'utf-8');
77+
const notFoundContent = await fs.readFile(notFoundPath, 'utf-8');
7778
expect(notFoundContent).toBe(indexContent);
7879
});
7980

8081
it('should NOT create 404.html when notfound is false', async () => {
8182
const indexPath = path.join(testDir, 'index.html');
82-
await fse.writeFile(indexPath, '<html><body>Test</body></html>');
83+
await fs.writeFile(indexPath, '<html><body>Test</body></html>');
8384

8485
const ghpages = require('gh-pages');
8586
jest.spyOn(ghpages, 'clean').mockImplementation(() => {});
@@ -94,7 +95,7 @@ describe('engine - real filesystem tests', () => {
9495
await engine.run(testDir, options, logger);
9596

9697
const notFoundPath = path.join(testDir, '404.html');
97-
const exists = await fse.pathExists(notFoundPath);
98+
const exists = await pathExists(notFoundPath);
9899
expect(exists).toBe(false);
99100
});
100101

@@ -122,13 +123,13 @@ describe('engine - real filesystem tests', () => {
122123
);
123124

124125
const notFoundPath = path.join(testDir, '404.html');
125-
const exists = await fse.pathExists(notFoundPath);
126+
const exists = await pathExists(notFoundPath);
126127
expect(exists).toBe(false);
127128
});
128129

129130
it('should NOT create 404.html when dry-run is true', async () => {
130131
const indexPath = path.join(testDir, 'index.html');
131-
await fse.writeFile(indexPath, '<html><body>Test</body></html>');
132+
await fs.writeFile(indexPath, '<html><body>Test</body></html>');
132133

133134
const ghpages = require('gh-pages');
134135
jest.spyOn(ghpages, 'clean').mockImplementation(() => {});
@@ -143,7 +144,7 @@ describe('engine - real filesystem tests', () => {
143144
await engine.run(testDir, options, logger);
144145

145146
const notFoundPath = path.join(testDir, '404.html');
146-
const exists = await fse.pathExists(notFoundPath);
147+
const exists = await pathExists(notFoundPath);
147148
expect(exists).toBe(false);
148149
});
149150
});
@@ -165,7 +166,7 @@ describe('engine - real filesystem tests', () => {
165166
describe('gh-pages v6 delegation - cname and nojekyll', () => {
166167
it('should pass cname option to gh-pages when provided', async () => {
167168
const indexPath = path.join(testDir, 'index.html');
168-
await fse.writeFile(indexPath, '<html>test</html>');
169+
await fs.writeFile(indexPath, '<html>test</html>');
169170

170171
const ghpages = require('gh-pages');
171172
jest.spyOn(ghpages, 'clean').mockImplementation(() => {});
@@ -194,7 +195,7 @@ describe('engine - real filesystem tests', () => {
194195

195196
it('should pass nojekyll option to gh-pages when enabled', async () => {
196197
const indexPath = path.join(testDir, 'index.html');
197-
await fse.writeFile(indexPath, '<html>test</html>');
198+
await fs.writeFile(indexPath, '<html>test</html>');
198199

199200
const ghpages = require('gh-pages');
200201
jest.spyOn(ghpages, 'clean').mockImplementation(() => {});
@@ -221,7 +222,7 @@ describe('engine - real filesystem tests', () => {
221222

222223
it('should pass both cname and nojekyll options when both enabled', async () => {
223224
const indexPath = path.join(testDir, 'index.html');
224-
await fse.writeFile(indexPath, '<html>test</html>');
225+
await fs.writeFile(indexPath, '<html>test</html>');
225226

226227
const ghpages = require('gh-pages');
227228
jest.spyOn(ghpages, 'clean').mockImplementation(() => {});
@@ -250,12 +251,12 @@ describe('engine - real filesystem tests', () => {
250251

251252
// Verify 404.html is still created by us (not delegated to gh-pages)
252253
const notFoundPath = path.join(testDir, '404.html');
253-
expect(await fse.pathExists(notFoundPath)).toBe(true);
254+
expect(await pathExists(notFoundPath)).toBe(true);
254255
});
255256

256257
it('should NOT pass cname when not provided (undefined)', async () => {
257258
const indexPath = path.join(testDir, 'index.html');
258-
await fse.writeFile(indexPath, '<html>test</html>');
259+
await fs.writeFile(indexPath, '<html>test</html>');
259260

260261
const ghpages = require('gh-pages');
261262
jest.spyOn(ghpages, 'clean').mockImplementation(() => {});
@@ -283,7 +284,7 @@ describe('engine - real filesystem tests', () => {
283284

284285
it('should pass nojekyll: false when disabled', async () => {
285286
const indexPath = path.join(testDir, 'index.html');
286-
await fse.writeFile(indexPath, '<html>test</html>');
287+
await fs.writeFile(indexPath, '<html>test</html>');
287288

288289
const ghpages = require('gh-pages');
289290
jest.spyOn(ghpages, 'clean').mockImplementation(() => {});

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

Lines changed: 8 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@
1212

1313
import { ChildProcess } from 'child_process';
1414

15+
import { pathExists } from '../utils';
16+
1517
const path = require('path');
16-
const fs = require('fs-extra');
18+
const fs = require('fs/promises');
1719
const os = require('os');
1820
const { EventEmitter } = require('events');
1921

@@ -141,7 +143,7 @@ describe('gh-pages v6.3.0 - behavioral snapshot', () => {
141143
// Create a real temp directory with test files
142144
tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gh-pages-test-'));
143145
basePath = path.join(tempDir, 'dist');
144-
await fs.ensureDir(basePath);
146+
await fs.mkdir(basePath, { recursive: true });
145147

146148
// Create test files (including dotfiles for testing dotfiles option)
147149
await fs.writeFile(path.join(basePath, 'index.html'), '<html>test</html>');
@@ -152,7 +154,8 @@ describe('gh-pages v6.3.0 - behavioral snapshot', () => {
152154

153155
afterAll(async () => {
154156
// Clean up temp directory
155-
await fs.remove(tempDir);
157+
// force: true matches fse.remove() behavior (no error if path doesn't exist)
158+
await fs.rm(tempDir, { recursive: true, force: true });
156159
});
157160

158161
beforeEach(() => {
@@ -1010,55 +1013,7 @@ describe('gh-pages v6.3.0 - behavioral snapshot', () => {
10101013
*/
10111014

10121015
/**
1013-
* gh-pages.clean() behavior
1014-
*
1015-
* What does clean() do?
1016-
* - Removes repo-specific cache subdirectories inside the gh-pages cache folder
1017-
* - Cache location: find-cache-dir determines base location (usually node_modules/.cache/gh-pages)
1018-
* - Structure: {cache-dir}/{filenamified-repo-url}/
1019-
* - Source: gh-pages lib/index.js
1020-
*
1021-
* Why this matters:
1022-
* - Fixes "Remote url mismatch" errors
1023-
* - Clears stale git state
1024-
* - angular-cli-ghpages calls this before every deployment
1025-
*
1026-
* Note: clean() removes repo-specific subdirectories, not the entire cache parent directory
1016+
* gh-pages.clean() behavior is tested in engine.gh-pages-clean.spec.ts
1017+
* That test uses real filesystem operations without mocks.
10271018
*/
1028-
describe('gh-pages.clean() behavior', () => {
1029-
it('should execute clean() without throwing errors', () => {
1030-
// clean() synchronously removes repo-specific cache directories
1031-
// This test verifies it doesn't throw - actual deletion is repo-specific
1032-
expect(() => ghPages.clean()).not.toThrow();
1033-
});
1034-
1035-
it('should remove repo-specific cache directory', async () => {
1036-
const findCacheDir = require('find-cache-dir');
1037-
const filenamify = require('filenamify');
1038-
1039-
const cacheBaseDir = findCacheDir({ name: 'gh-pages' });
1040-
if (!cacheBaseDir) {
1041-
// Skip if no cache dir available (e.g., in some CI environments)
1042-
return;
1043-
}
1044-
1045-
// Create a fake repo-specific cache directory
1046-
const fakeRepoUrl = 'https://github.com/test/clean-test-repo.git';
1047-
const repoCacheDir = path.join(cacheBaseDir, filenamify(fakeRepoUrl, { replacement: '!' }));
1048-
1049-
await fs.ensureDir(repoCacheDir);
1050-
await fs.writeFile(path.join(repoCacheDir, 'marker.txt'), 'should be deleted');
1051-
expect(await fs.pathExists(repoCacheDir)).toBe(true);
1052-
1053-
// Execute clean
1054-
ghPages.clean();
1055-
1056-
// Give filesystem time to process deletion
1057-
await new Promise(resolve => setTimeout(resolve, 100));
1058-
1059-
// Repo-specific directory should be removed
1060-
const existsAfter = await fs.pathExists(repoCacheDir);
1061-
expect(existsAfter).toBe(false);
1062-
});
1063-
});
10641019
});

‎src/engine/engine.gh-pages-clean.spec.ts‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/**
2+
* gh-pages.clean() behavior verification
3+
*
4+
* What we're testing:
5+
* - gh-pages.clean() actually removes cache directories
6+
* - This catches breaking changes if gh-pages updates its behavior
7+
*
8+
* Why maxWorkers: 1 is required:
9+
* gh-pages uses a globally shared cache at node_modules/.cache/gh-pages/
10+
* The clean() function is a nuclear option - it wipes the ENTIRE cache directory:
11+
*
12+
* exports.clean = function clean() {
13+
* fs.removeSync(getCacheDir()); // Removes ALL repo caches at once!
14+
* };
15+
*
16+
* If tests run in parallel, one test's clean() call destroys caches that
17+
* other tests are actively using, causing random failures.
18+
*/
19+
20+
import * as path from 'path';
21+
import * as fs from 'fs/promises';
22+
23+
import { pathExists } from '../utils';
24+
25+
const ghPages = require('gh-pages');
26+
const findCacheDir = require('find-cache-dir');
27+
const filenamify = require('filenamify');
28+
29+
describe('gh-pages.clean() behavior', () => {
30+
31+
it('should remove repo-specific cache directories', async () => {
32+
const cacheBaseDir = findCacheDir({ name: 'gh-pages' });
33+
if (!cacheBaseDir) {
34+
// Skip if no cache dir available (e.g., in some CI environments)
35+
console.warn('Skipping test: no gh-pages cache directory available');
36+
return;
37+
}
38+
39+
// Create a fake repo-specific cache directory
40+
const fakeRepoUrl = 'https://github.com/test/clean-test.git';
41+
const repoCacheDir = path.join(cacheBaseDir, filenamify(fakeRepoUrl, { replacement: '!' }));
42+
43+
// Setup: create the fake cache directory with a marker file
44+
await fs.mkdir(repoCacheDir, { recursive: true });
45+
await fs.writeFile(path.join(repoCacheDir, 'marker.txt'), 'should be deleted');
46+
expect(await pathExists(repoCacheDir)).toBe(true);
47+
48+
// Execute clean - this removes ALL repo cache directories
49+
ghPages.clean();
50+
51+
// Verify the directory was removed
52+
expect(await pathExists(repoCacheDir)).toBe(false);
53+
});
54+
55+
it('should not throw when cache directory does not exist', () => {
56+
// clean() should be safe to call even if nothing to clean
57+
expect(() => ghPages.clean()).not.toThrow();
58+
});
59+
});

0 commit comments

Comments
 (0)