diff --git a/src/spec-configuration/containerFeaturesOrder.ts b/src/spec-configuration/containerFeaturesOrder.ts index ac0bbf149..d143d5dd7 100644 --- a/src/spec-configuration/containerFeaturesOrder.ts +++ b/src/spec-configuration/containerFeaturesOrder.ts @@ -24,7 +24,8 @@ export function computeFeatureInstallationOrder(config: DevContainerConfig, feat } } -function computeOverrideInstallationOrder(config: DevContainerConfig, features: FeatureSet[]) { +// Exported for unit tests. +export function computeOverrideInstallationOrder(config: DevContainerConfig, features: FeatureSet[]) { // Starts with the automatic installation order. const automaticOrder = computeInstallationOrder(features); @@ -42,7 +43,8 @@ function computeOverrideInstallationOrder(config: DevContainerConfig, features: return orderedFeatures.concat(features); } -function computeInstallationOrder(features: FeatureSet[]) { +// Exported for unit tests. +export function computeInstallationOrder(features: FeatureSet[]) { const nodesById = features.map(feature => ({ feature, before: new Set(), diff --git a/src/test/container-features/containerFeaturesOrder.offline.test.ts b/src/test/container-features/containerFeaturesOrder.offline.test.ts new file mode 100644 index 000000000..9e99f7dda --- /dev/null +++ b/src/test/container-features/containerFeaturesOrder.offline.test.ts @@ -0,0 +1,113 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { ContainerError } from '../../spec-common/errors'; +import { FeatureSet } from '../../spec-configuration/containerFeaturesConfiguration'; +import { computeInstallationOrder, computeOverrideInstallationOrder } from '../../spec-configuration/containerFeaturesOrder'; +import { URI } from 'vscode-uri'; + +describe('Container features install order', () => { + + it('has stable order among independent features', () => { + assert.deepEqual( + computeInstallationOrder([ + installAfter('C'), + installAfter('A'), + installAfter('B'), + ]).map(f => f.features[0].id), + ['A', 'B', 'C'] + ); + }); + + it('orders "installAfter" first in breadth-first order (tree)', () => { + assert.deepEqual( + computeInstallationOrder([ + installAfter('A', 'B'), + installAfter('B', 'C'), + installAfter('C'), + installAfter('D', 'E'), + installAfter('E', 'C'), + ]).map(f => f.features[0].id), + ['C', 'B', 'E', 'A', 'D'] + ); + }); + + it('orders "installAfter" first in breadth-first order (DAG)', () => { + assert.deepEqual( + computeInstallationOrder([ + installAfter('A', 'B', 'C'), + installAfter('B', 'C'), + installAfter('C'), + installAfter('D', 'C'), + ]).map(f => f.features[0].id), + ['C', 'B', 'D', 'A'] + ); + }); + + it('treats "installAfter" is a soft dependency', () => { + assert.deepEqual( + computeInstallationOrder([ + installAfter('A', 'B', 'C'), + installAfter('B'), + ]).map(f => f.features[0].id), + ['B', 'A'] + ); + }); + + it('orders independent features last', () => { + assert.deepEqual( + computeInstallationOrder([ + installAfter('A'), + installAfter('B', 'C'), + installAfter('C'), + ]).map(f => f.features[0].id), + ['C', 'B', 'A'] + ); + }); + + it('detects cycles', () => { + try { + computeInstallationOrder([ + installAfter('A', 'B'), + installAfter('B'), + installAfter('C', 'D'), + installAfter('D', 'C'), + ]); + assert.fail('Cyclic dependency not detected.'); + } catch (err) { + assert.ok(err instanceof ContainerError); + assert.ok(err.message.indexOf('cyclic')); + } + }); + + it('respects OverrideConfig', () => { + assert.deepEqual( + computeOverrideInstallationOrder( + { image: 'ubuntu', configFilePath: URI.from({ 'scheme': 'https' }), overrideFeatureInstallOrder: ['A', 'B', 'C'] }, + [ + installAfter('A', 'C'), + installAfter('B', 'C'), + installAfter('C', 'D'), + ]).map(f => f.features[0].id), + ['A', 'B', 'C'] + ); + }); + + function installAfter(id: string, ...installAfter: string[]): FeatureSet { + return { + sourceInformation: { + type: 'local-cache', + }, + features: [{ + id, + name: id, + installAfter, + value: true, + included: true, + }], + }; + } +}); \ No newline at end of file diff --git a/src/test/container-features/generateFeaturesConfig.offline.test.ts b/src/test/container-features/generateFeaturesConfig.offline.test.ts index 0296e1866..bd3c3847e 100644 --- a/src/test/container-features/generateFeaturesConfig.offline.test.ts +++ b/src/test/container-features/generateFeaturesConfig.offline.test.ts @@ -4,7 +4,7 @@ import { createPlainLog, LogLevel, makeLog } from '../../spec-utils/log'; import * as os from 'os'; import * as path from 'path'; import { mkdirpLocal } from '../../spec-utils/pfs'; -import { DevContainerConfig, DevContainerFeature } from '../../spec-configuration/configuration'; +import { DevContainerConfig } from '../../spec-configuration/configuration'; import { URI } from 'vscode-uri'; export const output = makeLog(createPlainLog(text => process.stdout.write(text), () => LogLevel.Trace)); @@ -38,25 +38,18 @@ describe('validate (offline) generateFeaturesConfig()', function () { const tmpFolder: string = path.join(os.tmpdir(), 'vsch', 'container-features', `${version}-${Date.now()}`); await mkdirpLocal(tmpFolder); - const features: DevContainerFeature[] = [ - { - id: 'first', - options: { - 'version': 'latest' - }, - }, - { - id: 'second', - options: { - 'value': true - }, - } - ]; const config: DevContainerConfig = { configFilePath: URI.from({ 'scheme': 'https' }), dockerFile: '.', - features: features + features: { + first: { + 'version': 'latest' + }, + second: { + 'value': true + }, + }, }; const featuresConfig = await generateFeaturesConfig(params, tmpFolder, config, labels, localFeaturesFolder);