Skip to content

Commit b4fde66

Browse files
imnotjamesdkundel
imnotjames
authored and
dkundel
committed
feat: allow the same namespace but different files (#92)
* allow the same namespace but different files this allows you to install multiple templates under the same namepace * use existing fileExists function * move mkdir to utils/fs to simplify mocking * remove unused debugger * remove unused file-exists export * add tests for templating/filesystem
1 parent 9b2a981 commit b4fde66

File tree

4 files changed

+264
-21
lines changed

4 files changed

+264
-21
lines changed

‎__tests__/templating/filesystem.test.ts

+242
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,242 @@
1+
jest.mock('listr/lib/renderer');
2+
jest.mock('@twilio-labs/serverless-api');
3+
jest.mock('got');
4+
jest.mock('pkg-install');
5+
jest.mock('../../src/utils/fs');
6+
jest.mock('../../src/utils/logger');
7+
8+
import path from 'path';
9+
import { install } from 'pkg-install';
10+
import { fsHelpers } from '@twilio-labs/serverless-api';
11+
import got from 'got';
12+
import { mocked } from 'ts-jest/utils';
13+
import { writeFiles } from '../../src/templating/filesystem';
14+
import { downloadFile, fileExists, readFile, writeFile, mkdir } from '../../src/utils/fs';
15+
16+
beforeEach(() => {
17+
// For our test, replace the `listr` renderer with a silent one so the tests
18+
// don't get confusing output in them.
19+
const {getRenderer} = jest.requireMock('listr/lib/renderer');
20+
mocked(getRenderer).mockImplementation(() => require('listr-silent-renderer'));
21+
});
22+
23+
afterEach(() => {jest.resetAllMocks(); jest.restoreAllMocks() });
24+
25+
test('bubbles up an exception when functions directory is missing', async () => {
26+
// For this test, getFirstMatchingDirectory always errors
27+
mocked(fsHelpers.getFirstMatchingDirectory).mockImplementation((basePath: string, directories: Array<string>): string => {
28+
throw new Error(`Could not find any of these directories "${directories.join('", "')}"`);
29+
});
30+
31+
await expect(writeFiles([], './testing/', 'example'))
32+
.rejects.toThrowError('Could not find any of these directories "functions", "src"');
33+
});
34+
35+
test('bubbles up an exception when assets directory is missing', async () => {
36+
// For this test, getFirstMatchingDirectory only errors on `assets` directory.
37+
mocked(fsHelpers.getFirstMatchingDirectory).mockImplementation((basePath: string, directories: Array<string>): string => {
38+
if (directories.includes('functions')) {
39+
return path.join(basePath, directories[0]);
40+
}
41+
42+
throw new Error(`Could not find any of these directories "${directories.join('", "')}"`);
43+
});
44+
45+
await expect(writeFiles([], './testing/', 'example'))
46+
.rejects.toThrowError('Could not find any of these directories "assets", "static"');
47+
});
48+
49+
test('installation with basic functions', async () => {
50+
// For this test, getFirstMatchingDirectory never errors.
51+
mocked(fsHelpers.getFirstMatchingDirectory)
52+
.mockImplementation((basePath: string, directories: Array<string>): string => path.join(basePath, directories[0]));
53+
54+
await writeFiles(
55+
[
56+
{ name: 'hello.js', type: 'functions', content: 'https://example.com/hello.js' },
57+
{ name: '.env', type: '.env', content: 'https://example.com/.env' },
58+
],
59+
'./testing/',
60+
'example'
61+
);
62+
63+
expect(downloadFile).toHaveBeenCalledTimes(2);
64+
expect(downloadFile).toHaveBeenCalledWith('https://example.com/.env', 'testing/.env');
65+
expect(downloadFile).toHaveBeenCalledWith('https://example.com/hello.js', 'testing/functions/example/hello.js');
66+
67+
expect(mkdir).toHaveBeenCalledTimes(1);
68+
expect(mkdir).toHaveBeenCalledWith('testing/functions/example', {recursive: true});
69+
});
70+
71+
test('installation with functions and assets', async () => {
72+
// For this test, getFirstMatchingDirectory never errors.
73+
mocked(fsHelpers.getFirstMatchingDirectory)
74+
.mockImplementation((basePath: string, directories: Array<string>): string => path.join(basePath, directories[0]));
75+
76+
await writeFiles(
77+
[
78+
{ name: 'hello.js', type: 'functions', content: 'https://example.com/hello.js' },
79+
{ name: 'hello.wav', type: 'assets', content: 'https://example.com/hello.wav' },
80+
{ name: '.env', type: '.env', content: 'https://example.com/.env' },
81+
],
82+
'./testing/',
83+
'example'
84+
);
85+
86+
expect(downloadFile).toHaveBeenCalledTimes(3);
87+
expect(downloadFile).toHaveBeenCalledWith('https://example.com/.env', 'testing/.env');
88+
expect(downloadFile).toHaveBeenCalledWith('https://example.com/hello.js', 'testing/functions/example/hello.js');
89+
expect(downloadFile).toHaveBeenCalledWith('https://example.com/hello.wav', 'testing/assets/example/hello.wav');
90+
91+
expect(mkdir).toHaveBeenCalledTimes(2);
92+
expect(mkdir).toHaveBeenCalledWith('testing/functions/example', {recursive: true});
93+
expect(mkdir).toHaveBeenCalledWith('testing/assets/example', {recursive: true});
94+
});
95+
96+
test('installation without dot-env file causes unexpected crash', async () => {
97+
// I don't believe this is the intended behavior but it's the current behavior.
98+
// As such, let's create a test for it which can be removed / changed later
99+
// once the behavior is fixed.
100+
101+
// For this test, getFirstMatchingDirectory never errors.
102+
mocked(fsHelpers.getFirstMatchingDirectory)
103+
.mockImplementation((basePath: string, directories: Array<string>): string => path.join(basePath, directories[0]));
104+
105+
const expected = new TypeError('Cannot read property \'newEnvironmentVariableKeys\' of undefined');
106+
107+
await expect(writeFiles([], './testing/', 'example'))
108+
.rejects.toThrowError(expected);
109+
});
110+
111+
test('installation with an empty dependency file', async () => {
112+
// The typing of `got` is not exactly correct for this - it expects a
113+
// buffer but depending on inputs `got` can actually return an object.
114+
// @ts-ignore
115+
mocked(got).mockImplementation(() => Promise.resolve({ body: { dependencies: {} } }));
116+
117+
// For this test, getFirstMatchingDirectory never errors.
118+
mocked(fsHelpers.getFirstMatchingDirectory)
119+
.mockImplementation((basePath: string, directories: Array<string>): string => path.join(basePath, directories[0]));
120+
121+
await writeFiles(
122+
[
123+
{ name: 'package.json', type: 'package.json', content: 'https://example.com/package.json' },
124+
{ name: '.env', type: '.env', content: 'https://example.com/.env' },
125+
],
126+
'./testing/',
127+
'example'
128+
);
129+
130+
expect(downloadFile).toHaveBeenCalledTimes(1);
131+
expect(downloadFile).toHaveBeenCalledWith('https://example.com/.env', 'testing/.env');
132+
133+
expect(got).toHaveBeenCalledTimes(1);
134+
expect(got).toHaveBeenCalledWith('https://example.com/package.json', { json: true });
135+
136+
expect(install).not.toHaveBeenCalled();
137+
});
138+
139+
test('installation with a dependency file', async () => {
140+
// The typing of `got` is not exactly correct for this - it expects a
141+
// buffer but depending on inputs `got` can actually return an object.
142+
// @ts-ignore
143+
mocked(got).mockImplementation(() => Promise.resolve({ body: { dependencies: {foo: '^1.0.0', got: '^6.9.0'} } }));
144+
145+
// For this test, getFirstMatchingDirectory never errors.
146+
mocked(fsHelpers.getFirstMatchingDirectory)
147+
.mockImplementation((basePath: string, directories: Array<string>): string => path.join(basePath, directories[0]));
148+
149+
await writeFiles(
150+
[
151+
{ name: 'package.json', type: 'package.json', content: 'https://example.com/package.json' },
152+
{ name: '.env', type: '.env', content: 'https://example.com/.env' },
153+
],
154+
'./testing/',
155+
'example'
156+
);
157+
158+
expect(downloadFile).toHaveBeenCalledTimes(1);
159+
expect(downloadFile).toHaveBeenCalledWith('https://example.com/.env', 'testing/.env');
160+
161+
expect(got).toHaveBeenCalledTimes(1);
162+
expect(got).toHaveBeenCalledWith('https://example.com/package.json', { json: true });
163+
164+
expect(install).toHaveBeenCalledTimes(1);
165+
expect(install).toHaveBeenCalledWith({foo: '^1.0.0', got: '^6.9.0'}, {cwd: './testing/'});
166+
});
167+
168+
test('installation with an existing dot-env file', async () => {
169+
mocked(fileExists).mockReturnValue(Promise.resolve(true));
170+
mocked(readFile).mockReturnValue(Promise.resolve('# Comment\nFOO=BAR\n'));
171+
172+
// The typing of `got` is not exactly correct for this - it expects a
173+
// buffer but depending on inputs `got` can actually return an object.
174+
// @ts-ignore
175+
mocked(got).mockImplementation(() => Promise.resolve({ body: 'HELLO=WORLD\n' }));
176+
177+
// For this test, getFirstMatchingDirectory never errors.
178+
mocked(fsHelpers.getFirstMatchingDirectory)
179+
.mockImplementation((basePath: string, directories: Array<string>): string => path.join(basePath, directories[0]));
180+
181+
await writeFiles(
182+
[
183+
{ name: '.env', type: '.env', content: 'https://example.com/.env' },
184+
],
185+
'./testing/',
186+
'example'
187+
);
188+
189+
expect(downloadFile).toHaveBeenCalledTimes(0);
190+
191+
expect(writeFile).toHaveBeenCalledTimes(1);
192+
expect(writeFile).toHaveBeenCalledWith(
193+
'testing/.env',
194+
'# Comment\n' +
195+
'FOO=BAR\n' +
196+
'\n\n' +
197+
'# Variables for function \".env\"\n' + // This seems to be a bug but is the output.
198+
'# ---\n' +
199+
'HELLO=WORLD\n',
200+
"utf8"
201+
);
202+
});
203+
204+
test('installation with overlapping function files throws errors before writing', async () => {
205+
mocked(fsHelpers.getFirstMatchingDirectory)
206+
.mockImplementation((basePath: string, directories: Array<string>): string => path.join(basePath, directories[0]));
207+
208+
mocked(fileExists).mockImplementation((p) => Promise.resolve(p == 'functions/example/hello.js'));
209+
210+
await expect(writeFiles(
211+
[
212+
{ name: 'hello.js', type: 'functions', content: 'https://example.com/hello.js' },
213+
{ name: '.env', type: '.env', content: 'https://example.com/.env' },
214+
],
215+
'./',
216+
'example'
217+
)).rejects.toThrowError('Template with name "example" has duplicate file "hello.js" in "functions"');
218+
219+
expect(downloadFile).toHaveBeenCalledTimes(0);
220+
expect(writeFile).toHaveBeenCalledTimes(0);
221+
222+
});
223+
224+
test('installation with overlapping asset files throws errors before writing', async () => {
225+
mocked(fsHelpers.getFirstMatchingDirectory)
226+
.mockImplementation((basePath: string, directories: Array<string>): string => path.join(basePath, directories[0]));
227+
228+
mocked(fileExists).mockImplementation((p) => Promise.resolve(p == 'assets/example/hello.wav'));
229+
230+
await expect(writeFiles(
231+
[
232+
{ name: 'hello.js', type: 'functions', content: 'https://example.com/hello.js' },
233+
{ name: 'hello.wav', type: 'assets', content: 'https://example.com/hello.wav' },
234+
{ name: '.env', type: '.env', content: 'https://example.com/.env' },
235+
],
236+
'./',
237+
'example'
238+
)).rejects.toThrowError('Template with name "example" has duplicate file "hello.wav" in "assets"');
239+
240+
expect(downloadFile).toHaveBeenCalledTimes(0);
241+
expect(writeFile).toHaveBeenCalledTimes(0);
242+
});

‎package.json

+1
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@
102102
"jest": "^24.8.0",
103103
"jest-express": "^1.10.1",
104104
"lint-staged": "^8.2.1",
105+
"listr-silent-renderer": "^1.1.1",
105106
"npm-run-all": "^4.1.5",
106107
"prettier": "^1.18.2",
107108
"rimraf": "^2.6.3",

‎src/templating/filesystem.ts

+20-21
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,13 @@
11
import { fsHelpers } from '@twilio-labs/serverless-api';
22
import chalk from 'chalk';
33
import dotenv from 'dotenv';
4-
import { mkdir as oldMkdir } from 'fs';
54
import got from 'got';
65
import Listr, { ListrTask } from 'listr';
76
import path from 'path';
87
import { install, InstallResult } from 'pkg-install';
9-
import { promisify } from 'util';
10-
import { downloadFile, fileExists, readFile, writeFile } from '../utils/fs';
11-
import { getDebugFunction, logger } from '../utils/logger';
8+
import { downloadFile, fileExists, readFile, writeFile, mkdir } from '../utils/fs';
9+
import { logger } from '../utils/logger';
1210
import { TemplateFileInfo } from './data';
13-
const mkdir = promisify(oldMkdir);
14-
15-
const debug = getDebugFunction('twilio-run:templating:filesystem');
1611

1712
async function writeEnvFile(
1813
contentUrl: string,
@@ -99,23 +94,29 @@ export async function writeFiles(
9994

10095
if (functionsTargetDir !== functionsDir) {
10196
if (hasFilesOfType(files, 'functions')) {
102-
try {
103-
await mkdir(functionsTargetDir);
104-
} catch (err) {
105-
debug(err);
97+
await mkdir(functionsTargetDir, { recursive: true });
98+
}
99+
100+
if (hasFilesOfType(files, 'assets')) {
101+
await mkdir(assetsTargetDir, { recursive: true });
102+
}
103+
}
104+
105+
for (let file of files) {
106+
if (file.type === 'functions') {
107+
let filepath = path.join(functionsTargetDir, file.name);
108+
109+
if (await fileExists(filepath)) {
106110
throw new Error(
107-
`Template with name "${namespace}" already exists in "${functionsDir}"`
111+
`Template with name "${namespace}" has duplicate file "${file.name}" in "${functionsDir}"`
108112
);
109113
}
110-
}
114+
} else if (file.type === 'assets') {
115+
let filepath = path.join(assetsTargetDir, file.name);
111116

112-
if (hasFilesOfType(files, 'assets')) {
113-
try {
114-
await mkdir(assetsTargetDir);
115-
} catch (err) {
116-
debug(err);
117+
if (await fileExists(filepath)) {
117118
throw new Error(
118-
`Template with name "${namespace}" already exists in "${assetsDir}"`
119+
`Template with name "${namespace}" has duplicate file "${file.name}" in "${assetsDir}"`
119120
);
120121
}
121122
}
@@ -169,5 +170,3 @@ export async function writeFiles(
169170
);
170171
}
171172
}
172-
173-
export { fileExists } from '../utils/fs';

‎src/utils/fs.ts

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const access = promisify(fs.access);
77
export const readFile = promisify(fs.readFile);
88
export const writeFile = promisify(fs.writeFile);
99
export const readdir = promisify(fs.readdir);
10+
export const mkdir = promisify(fs.mkdir);
1011
const stat = promisify(fs.stat);
1112
const open = promisify(fs.open);
1213

0 commit comments

Comments
 (0)