Skip to content

Commit 6f7032f

Browse files
JamieSlomeJamieSlome
authored
Merge pull request finos#1032 from jescalada/improve-config-test-coverage
test: improve `config` test coverage
2 parents a3834ab + c69427b commit 6f7032f

File tree

3 files changed

+389
-33
lines changed

3 files changed

+389
-33
lines changed

‎src/config/ConfigLoader.ts‎

Lines changed: 15 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,8 @@ export class ConfigLoader extends EventEmitter {
196196
try {
197197
console.log(`Loading configuration from ${source.type} source`);
198198
return await this.loadFromSource(source);
199-
} catch (error: unknown) {
200-
if (error instanceof Error) {
201-
console.error(`Error loading from ${source.type} source:`, error.message);
202-
}
199+
} catch (error: any) {
200+
console.error(`Error loading from ${source.type} source:`, error.message);
203201
return null;
204202
}
205203
}),
@@ -234,7 +232,7 @@ export class ConfigLoader extends EventEmitter {
234232
} else {
235233
console.log('Configuration has not changed, no update needed');
236234
}
237-
} catch (error: unknown) {
235+
} catch (error: any) {
238236
console.error('Error reloading configuration:', error);
239237
this.emit('configurationError', error);
240238
} finally {
@@ -330,24 +328,18 @@ export class ConfigLoader extends EventEmitter {
330328
try {
331329
await execFileAsync('git', ['clone', source.repository, repoDir], execOptions);
332330
console.log('Repository cloned successfully');
333-
} catch (error: unknown) {
334-
if (error instanceof Error) {
335-
console.error('Failed to clone repository:', error.message);
336-
throw new Error(`Failed to clone repository: ${error.message}`);
337-
}
338-
throw error;
331+
} catch (error: any) {
332+
console.error('Failed to clone repository:', error.message);
333+
throw new Error(`Failed to clone repository: ${error.message}`);
339334
}
340335
} else {
341336
console.log(`Pulling latest changes from ${source.repository}`);
342337
try {
343338
await execFileAsync('git', ['pull'], { cwd: repoDir });
344339
console.log('Repository pulled successfully');
345-
} catch (error: unknown) {
346-
if (error instanceof Error) {
347-
console.error('Failed to pull repository:', error.message);
348-
throw new Error(`Failed to pull repository: ${error.message}`);
349-
}
350-
throw error;
340+
} catch (error: any) {
341+
console.error('Failed to pull repository:', error.message);
342+
throw new Error(`Failed to pull repository: ${error.message}`);
351343
}
352344
}
353345

@@ -357,12 +349,9 @@ export class ConfigLoader extends EventEmitter {
357349
try {
358350
await execFileAsync('git', ['checkout', source.branch], { cwd: repoDir });
359351
console.log(`Branch ${source.branch} checked out successfully`);
360-
} catch (error: unknown) {
361-
if (error instanceof Error) {
362-
console.error(`Failed to checkout branch ${source.branch}:`, error.message);
363-
throw new Error(`Failed to checkout branch ${source.branch}: ${error.message}`);
364-
}
365-
throw error;
352+
} catch (error: any) {
353+
console.error(`Failed to checkout branch ${source.branch}:`, error.message);
354+
throw new Error(`Failed to checkout branch ${source.branch}: ${error.message}`);
366355
}
367356
}
368357

@@ -382,12 +371,9 @@ export class ConfigLoader extends EventEmitter {
382371
const config = JSON.parse(content);
383372
console.log('Configuration loaded successfully from Git');
384373
return config;
385-
} catch (error: unknown) {
386-
if (error instanceof Error) {
387-
console.error('Failed to read or parse configuration file:', error.message);
388-
throw new Error(`Failed to read or parse configuration file: ${error.message}`);
389-
}
390-
throw error;
374+
} catch (error: any) {
375+
console.error('Failed to read or parse configuration file:', error.message);
376+
throw new Error(`Failed to read or parse configuration file: ${error.message}`);
391377
}
392378
}
393379

‎test/ConfigLoader.test.js‎

Lines changed: 246 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,22 @@ describe('ConfigLoader', () => {
148148

149149
expect(spy.calledOnce).to.be.true; // Should only emit once
150150
});
151+
152+
it('should not emit event if configurationSources is disabled', async () => {
153+
const config = {
154+
configurationSources: {
155+
enabled: false,
156+
},
157+
};
158+
159+
configLoader = new ConfigLoader(config);
160+
const spy = sinon.spy();
161+
configLoader.on('configurationChanged', spy);
162+
163+
await configLoader.reloadConfiguration();
164+
165+
expect(spy.called).to.be.false;
166+
});
151167
});
152168

153169
describe('initialize', () => {
@@ -182,6 +198,98 @@ describe('ConfigLoader', () => {
182198
});
183199
});
184200

201+
describe('start', () => {
202+
it('should perform initial load on start if configurationSources is enabled', async () => {
203+
const mockConfig = {
204+
configurationSources: {
205+
enabled: true,
206+
sources: [
207+
{
208+
type: 'file',
209+
enabled: true,
210+
path: tempConfigFile,
211+
},
212+
],
213+
reloadIntervalSeconds: 30,
214+
},
215+
};
216+
217+
const configLoader = new ConfigLoader(mockConfig);
218+
const spy = sinon.spy(configLoader, 'reloadConfiguration');
219+
await configLoader.start();
220+
221+
expect(spy.calledOnce).to.be.true;
222+
});
223+
224+
it('should clear an existing reload interval if it exists', async () => {
225+
const mockConfig = {
226+
configurationSources: {
227+
enabled: true,
228+
sources: [
229+
{
230+
type: 'file',
231+
enabled: true,
232+
path: tempConfigFile,
233+
},
234+
],
235+
},
236+
};
237+
238+
const configLoader = new ConfigLoader(mockConfig);
239+
configLoader.reloadTimer = setInterval(() => {}, 1000);
240+
await configLoader.start();
241+
242+
expect(configLoader.reloadTimer).to.be.null;
243+
});
244+
245+
it('should run reloadConfiguration multiple times on short reload interval', async () => {
246+
const mockConfig = {
247+
configurationSources: {
248+
enabled: true,
249+
sources: [
250+
{
251+
type: 'file',
252+
enabled: true,
253+
path: tempConfigFile,
254+
},
255+
],
256+
reloadIntervalSeconds: 0.01,
257+
},
258+
};
259+
260+
const configLoader = new ConfigLoader(mockConfig);
261+
const spy = sinon.spy(configLoader, 'reloadConfiguration');
262+
await configLoader.start();
263+
264+
// Make sure the reload interval is triggered
265+
await new Promise((resolve) => setTimeout(resolve, 50));
266+
267+
expect(spy.callCount).to.greaterThan(1);
268+
});
269+
270+
it('should clear the interval when stop is called', async () => {
271+
const mockConfig = {
272+
configurationSources: {
273+
enabled: true,
274+
sources: [
275+
{
276+
type: 'file',
277+
enabled: true,
278+
path: tempConfigFile,
279+
},
280+
],
281+
},
282+
};
283+
284+
const configLoader = new ConfigLoader(mockConfig);
285+
configLoader.reloadTimer = setInterval(() => {}, 1000);
286+
expect(configLoader.reloadTimer).to.not.be.null;
287+
288+
await configLoader.stop();
289+
expect(configLoader.reloadTimer).to.be.null;
290+
});
291+
});
292+
185293
describe('loadRemoteConfig', () => {
186294
let configLoader;
187295
beforeEach(async () => {
@@ -205,15 +313,15 @@ describe('ConfigLoader', () => {
205313
enabled: true,
206314
};
207315

208-
const config = await configLoader.loadFromGit(source);
316+
const config = await configLoader.loadFromSource(source);
209317

210318
// Verify the loaded config has expected structure
211319
expect(config).to.be.an('object');
212320
expect(config).to.have.property('proxyUrl');
213321
expect(config).to.have.property('cookieSecret');
214322
});
215323

216-
it('should throw error for invalid configuration file path', async function () {
324+
it('should throw error for invalid configuration file path (git)', async function () {
217325
const source = {
218326
type: 'git',
219327
repository: 'https://github.com/finos/git-proxy.git',
@@ -223,13 +331,28 @@ describe('ConfigLoader', () => {
223331
};
224332

225333
try {
226-
await configLoader.loadFromGit(source);
334+
await configLoader.loadFromSource(source);
227335
throw new Error('Expected error was not thrown');
228336
} catch (error) {
229337
expect(error.message).to.equal('Invalid configuration file path in repository');
230338
}
231339
});
232340

341+
it('should throw error for invalid configuration file path (file)', async function () {
342+
const source = {
343+
type: 'file',
344+
path: '\0', // Invalid path
345+
enabled: true,
346+
};
347+
348+
try {
349+
await configLoader.loadFromSource(source);
350+
throw new Error('Expected error was not thrown');
351+
} catch (error) {
352+
expect(error.message).to.equal('Invalid configuration file path');
353+
}
354+
});
355+
233356
it('should load configuration from http', async function () {
234357
// eslint-disable-next-line no-invalid-this
235358
this.timeout(10000);
@@ -240,13 +363,132 @@ describe('ConfigLoader', () => {
240363
enabled: true,
241364
};
242365

243-
const config = await configLoader.loadFromHttp(source);
366+
const config = await configLoader.loadFromSource(source);
244367

245368
// Verify the loaded config has expected structure
246369
expect(config).to.be.an('object');
247370
expect(config).to.have.property('proxyUrl');
248371
expect(config).to.have.property('cookieSecret');
249372
});
373+
374+
it('should throw error if repository is invalid', async function () {
375+
const source = {
376+
type: 'git',
377+
repository: 'invalid-repository',
378+
path: 'proxy.config.json',
379+
branch: 'main',
380+
enabled: true,
381+
};
382+
383+
try {
384+
await configLoader.loadFromSource(source);
385+
throw new Error('Expected error was not thrown');
386+
} catch (error) {
387+
expect(error.message).to.equal('Invalid repository URL format');
388+
}
389+
});
390+
391+
it('should throw error if branch name is invalid', async function () {
392+
const source = {
393+
type: 'git',
394+
repository: 'https://github.com/finos/git-proxy.git',
395+
path: 'proxy.config.json',
396+
branch: '..', // invalid branch pattern
397+
enabled: true,
398+
};
399+
400+
try {
401+
await configLoader.loadFromSource(source);
402+
throw new Error('Expected error was not thrown');
403+
} catch (error) {
404+
expect(error.message).to.equal('Invalid branch name format');
405+
}
406+
});
407+
408+
it('should throw error if configuration source is invalid', async function () {
409+
const source = {
410+
type: 'invalid',
411+
repository: 'https://github.com/finos/git-proxy.git',
412+
path: 'proxy.config.json',
413+
branch: 'main',
414+
enabled: true,
415+
};
416+
417+
try {
418+
await configLoader.loadFromSource(source);
419+
throw new Error('Expected error was not thrown');
420+
} catch (error) {
421+
expect(error.message).to.contain('Unsupported configuration source type');
422+
}
423+
});
424+
425+
it('should throw error if repository is a valid URL but not a git repository', async function () {
426+
const source = {
427+
type: 'git',
428+
repository: 'https://github.com/test-org/test-repo.git',
429+
path: 'proxy.config.json',
430+
branch: 'main',
431+
enabled: true,
432+
};
433+
434+
try {
435+
await configLoader.loadFromSource(source);
436+
throw new Error('Expected error was not thrown');
437+
} catch (error) {
438+
expect(error.message).to.contain('Failed to clone repository');
439+
}
440+
});
441+
442+
it('should throw error if repository is a valid git repo but the branch does not exist', async function () {
443+
const source = {
444+
type: 'git',
445+
repository: 'https://github.com/finos/git-proxy.git',
446+
path: 'proxy.config.json',
447+
branch: 'branch-does-not-exist',
448+
enabled: true,
449+
};
450+
451+
try {
452+
await configLoader.loadFromSource(source);
453+
throw new Error('Expected error was not thrown');
454+
} catch (error) {
455+
expect(error.message).to.contain('Failed to checkout branch');
456+
}
457+
});
458+
459+
it('should throw error if config path was not found', async function () {
460+
const source = {
461+
type: 'git',
462+
repository: 'https://github.com/finos/git-proxy.git',
463+
path: 'path-not-found.json',
464+
branch: 'main',
465+
enabled: true,
466+
};
467+
468+
try {
469+
await configLoader.loadFromSource(source);
470+
throw new Error('Expected error was not thrown');
471+
} catch (error) {
472+
expect(error.message).to.contain('Configuration file not found at');
473+
}
474+
});
475+
476+
it('should throw error if config file is not valid JSON', async function () {
477+
const source = {
478+
type: 'git',
479+
repository: 'https://github.com/finos/git-proxy.git',
480+
path: 'test/fixtures/baz.js',
481+
branch: 'main',
482+
enabled: true,
483+
};
484+
485+
try {
486+
await configLoader.loadFromSource(source);
487+
throw new Error('Expected error was not thrown');
488+
} catch (error) {
489+
expect(error.message).to.contain('Failed to read or parse configuration file');
490+
}
491+
});
250492
});
251493

252494
describe('deepMerge', () => {

0 commit comments

Comments
 (0)