Add tests for ConfigJsonConfigBackend

Also deprecate path-getting method, and remove OS version check.
The OS version itself is not used in ConfigJsonConfigBackend, so
it seems the OS version check is to confirm the existence of config.json
during class init, because OS version is a field that's always there
in a valid config.json.

Signed-off-by: Christina Ying Wang <christina@balena.io>

src/config/configJson.ts

@@ -3,7 +3,6 @@ import _ from 'lodash';
 
 import * as constants from '../lib/constants';
 import * as hostUtils from '../lib/host-utils';
-import * as osRelease from '../lib/os-release';
 import { takeGlobalLockRO, takeGlobalLockRW } from '../lib/process-lock';
 import type * as Schema from './schema';
 
@@ -12,17 +11,20 @@ export default class ConfigJsonConfigBackend {
 	private readonly writeLockConfigJson: () => Bluebird.Disposer<() => void>;
 
 	private readonly schema: Schema.Schema;
+	/**
+	 * @deprecated configPath is only set by legacy tests
+	 */
 	private readonly configPath?: string;
 
 	private cache: { [key: string]: unknown } = {};
 
-	private readonly init = _.once(async () =>
-		Object.assign(this.cache, await this.read()),
-	);
+	private readonly init = _.once(async () => {
+		Object.assign(this.cache, await this.read());
+	});
 
 	public constructor(schema: Schema.Schema, configPath?: string) {
-		this.configPath = configPath;
 		this.schema = schema;
+		this.configPath = configPath;
 
 		this.writeLockConfigJson = () =>
 			takeGlobalLockRW('config.json').disposer((release) => release());
@@ -91,6 +93,12 @@ export default class ConfigJsonConfigBackend {
 		return JSON.parse(await hostUtils.readFromBoot(filename, 'utf-8'));
 	}
 
+	/**
+	 * @deprecated Either read the config.json path from lib/constants, or
+	 * pass a validated path to the constructor and fail if no path is passed.
+	 * TODO: Remove this once api-binder tests are migrated. The only
+	 * time configPath is passed to the constructor is in the legacy tests.
+	 */
 	private async path(): Promise<string> {
 		// TODO: Remove this once api-binder tests are migrated. The only
 		// time configPath is passed to the constructor is in the legacy tests.
@@ -98,11 +106,6 @@ export default class ConfigJsonConfigBackend {
 			return this.configPath;
 		}
 
-		const osVersion = await osRelease.getOSVersion(constants.hostOSVersionPath);
-		if (osVersion == null) {
-			throw new Error('Failed to detect OS version!');
-		}
-
 		// The default path in the boot partition
 		return constants.configJsonPath;
 	}

test/integration/config/configJson.spec.ts

@@ -0,0 +1,101 @@
+import { expect } from 'chai';
+import { testfs } from 'mocha-pod';
+import { promises as fs } from 'fs';
+
+import ConfigJsonConfigBackend from '~/src/config/configJson';
+import { schema } from '~/src/config/schema';
+
+describe('ConfigJsonConfigBackend', () => {
+	const CONFIG_PATH = '/mnt/boot/config.json';
+	let configJsonConfigBackend: ConfigJsonConfigBackend;
+
+	beforeEach(() => {
+		configJsonConfigBackend = new ConfigJsonConfigBackend(schema);
+	});
+
+	it('should get value for config.json key', async () => {
+		await testfs({
+			[CONFIG_PATH]: JSON.stringify({
+				apiEndpoint: 'foo',
+			}),
+			'/mnt/root/etc/os-release': testfs.from('test/data/etc/os-release'),
+		}).enable();
+
+		expect(await configJsonConfigBackend.get('apiEndpoint')).to.equal('foo');
+
+		await testfs.restore();
+	});
+
+	it('should set value for config.json key', async () => {
+		await testfs({
+			[CONFIG_PATH]: JSON.stringify({
+				apiEndpoint: 'foo',
+			}),
+			'/mnt/root/etc/os-release': testfs.from('test/data/etc/os-release'),
+		}).enable();
+
+		await configJsonConfigBackend.set({
+			apiEndpoint: 'bar',
+		});
+
+		expect(await configJsonConfigBackend.get('apiEndpoint')).to.equal('bar');
+
+		await testfs.restore();
+	});
+
+	// The following test cases may be unnecessary as they test cases where another party
+	// writes to config.json directly (instead of through setting config vars on the API).
+	it('should get cached value even if actual value has changed', async () => {
+		await testfs({
+			[CONFIG_PATH]: JSON.stringify({
+				apiEndpoint: 'foo',
+			}),
+			'/mnt/root/etc/os-release': testfs.from('test/data/etc/os-release'),
+		}).enable();
+
+		// The cached value should be returned
+		expect(await configJsonConfigBackend.get('apiEndpoint')).to.equal('foo');
+
+		// Change the value in the file
+		await fs.writeFile(
+			CONFIG_PATH,
+			JSON.stringify({
+				apiEndpoint: 'bar',
+			}),
+		);
+
+		// Unintended behavior: the cached value should not be overwritten
+		expect(await configJsonConfigBackend.get('apiEndpoint')).to.equal('foo');
+
+		await testfs.restore();
+	});
+
+	it('should set value and refresh cache to equal new value', async () => {
+		await testfs({
+			[CONFIG_PATH]: JSON.stringify({
+				apiEndpoint: 'foo',
+			}),
+			'/mnt/root/etc/os-release': testfs.from('test/data/etc/os-release'),
+		}).enable();
+
+		expect(await configJsonConfigBackend.get('apiEndpoint')).to.equal('foo');
+
+		await fs.writeFile(
+			CONFIG_PATH,
+			JSON.stringify({
+				apiEndpoint: 'bar',
+			}),
+		);
+
+		// Unintended behavior: cached value should not have been updated
+		expect(await configJsonConfigBackend.get('apiEndpoint')).to.equal('foo');
+
+		await configJsonConfigBackend.set({
+			apiEndpoint: 'baz',
+		});
+
+		expect(await configJsonConfigBackend.get('apiEndpoint')).to.equal('baz');
+
+		await testfs.restore();
+	});
+});