From f7cecce7f2a5a4b4c8d2a19a0cb4c2750e6a92d2 Mon Sep 17 00:00:00 2001 From: Thorarinn Sigurdsson Date: Mon, 22 Apr 2019 19:39:21 +0200 Subject: [PATCH] refactor: rename task baseKey to key and key to id In short, baseKey is now called key, key is now called id, and id is now called uid. The same applies to the corresponding getter methods on BaseTask. baseKey has always been a somewhat awkward term, and with some of the recent changes to the events emitted by the TaskGraph, it had started to become slightly confusing as well. The new terminology should be clearer and more intuitive. --- garden-service/package-lock.json | 52 ++------ garden-service/src/commands/run/task.ts | 2 +- garden-service/src/logger/log-entry.ts | 4 +- garden-service/src/process.ts | 2 +- garden-service/src/task-graph.ts | 126 +++++++++--------- garden-service/src/tasks/base.ts | 10 +- garden-service/src/tasks/helpers.ts | 2 +- garden-service/test/integ-helpers.ts | 22 +-- .../test/integ/src/integ-helpers.ts | 16 +-- garden-service/test/integ/src/pre-release.ts | 20 +++ garden-service/test/run-garden.ts | 9 +- garden-service/test/unit/src/task-graph.ts | 46 +++---- garden-service/test/unit/src/tasks/helpers.ts | 2 +- 13 files changed, 156 insertions(+), 157 deletions(-) diff --git a/garden-service/package-lock.json b/garden-service/package-lock.json index 4dcb0f7e4a..82fb85ca66 100644 --- a/garden-service/package-lock.json +++ b/garden-service/package-lock.json @@ -3808,8 +3808,7 @@ "ansi-regex": { "version": "2.1.1", "resolved": false, - "integrity": "sha1-w7M6te42DYbg5ijwRorn7yfWVN8=", - "optional": true + "integrity": "sha1-w7M6te42DYbg5ijwRorn7yfWVN8=" }, "aproba": { "version": "1.2.0", @@ -3830,14 +3829,12 @@ "balanced-match": { "version": "1.0.0", "resolved": false, - "integrity": "sha1-ibTRmasr7kneFk6gK4nORi1xt2c=", - "optional": true + "integrity": "sha1-ibTRmasr7kneFk6gK4nORi1xt2c=" }, "brace-expansion": { "version": "1.1.11", "resolved": false, "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", - "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -3852,20 +3849,17 @@ "code-point-at": { "version": "1.1.0", "resolved": false, - "integrity": "sha1-DQcLTQQ6W+ozovGkDi7bPZpMz3c=", - "optional": true + "integrity": "sha1-DQcLTQQ6W+ozovGkDi7bPZpMz3c=" }, "concat-map": { "version": "0.0.1", "resolved": false, - "integrity": "sha1-2Klr13/Wjfd5OnMDajug1UBdR3s=", - "optional": true + "integrity": "sha1-2Klr13/Wjfd5OnMDajug1UBdR3s=" }, "console-control-strings": { "version": "1.1.0", "resolved": false, - "integrity": "sha1-PXz0Rk22RG6mRL9LOVB/mFEAjo4=", - "optional": true + "integrity": "sha1-PXz0Rk22RG6mRL9LOVB/mFEAjo4=" }, "core-util-is": { "version": "1.0.2", @@ -3982,8 +3976,7 @@ "inherits": { "version": "2.0.3", "resolved": false, - "integrity": "sha1-Yzwsg+PaQqUC9SRmAiSA9CCCYd4=", - "optional": true + "integrity": "sha1-Yzwsg+PaQqUC9SRmAiSA9CCCYd4=" }, "ini": { "version": "1.3.5", @@ -3995,7 +3988,6 @@ "version": "1.0.0", "resolved": false, "integrity": "sha1-754xOG8DGn8NZDr4L95QxFfvAMs=", - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -4010,7 +4002,6 @@ "version": "3.0.4", "resolved": false, "integrity": "sha512-yJHVQEhyqPLUTgt9B83PXu6W3rx4MvvHvSUvToogpwoGDOUQ+yDrR0HRot+yOCdCO7u4hX3pWft6kWBBcqh0UA==", - "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -4018,14 +4009,12 @@ "minimist": { "version": "0.0.8", "resolved": false, - "integrity": "sha1-hX/Kv8M5fSYluCKCYuhqp6ARsF0=", - "optional": true + "integrity": "sha1-hX/Kv8M5fSYluCKCYuhqp6ARsF0=" }, "minipass": { "version": "2.2.4", "resolved": false, "integrity": "sha512-hzXIWWet/BzWhYs2b+u7dRHlruXhwdgvlTMDKC6Cb1U7ps6Ac6yQlR39xsbjWJE377YTCtKwIXIpJ5oP+j5y8g==", - "optional": true, "requires": { "safe-buffer": "^5.1.1", "yallist": "^3.0.0" @@ -4044,7 +4033,6 @@ "version": "0.5.1", "resolved": false, "integrity": "sha1-MAV0OOrGz3+MR2fzhkjWaX11yQM=", - "optional": true, "requires": { "minimist": "0.0.8" } @@ -4125,8 +4113,7 @@ "number-is-nan": { "version": "1.0.1", "resolved": false, - "integrity": "sha1-CXtgK1NCKlIsGvuHkDGDNpQaAR0=", - "optional": true + "integrity": "sha1-CXtgK1NCKlIsGvuHkDGDNpQaAR0=" }, "object-assign": { "version": "4.1.1", @@ -4138,7 +4125,6 @@ "version": "1.4.0", "resolved": false, "integrity": "sha1-WDsap3WWHUsROsF9nFC6753Xa9E=", - "optional": true, "requires": { "wrappy": "1" } @@ -4224,8 +4210,7 @@ "safe-buffer": { "version": "5.1.1", "resolved": false, - "integrity": "sha512-kKvNJn6Mm93gAczWVJg7wH+wGYWNrDHdWvpUmHyEsgCtIwwo3bqPtV4tR5tuPaUhTOo/kvhVwd8XwwOllGYkbg==", - "optional": true + "integrity": "sha512-kKvNJn6Mm93gAczWVJg7wH+wGYWNrDHdWvpUmHyEsgCtIwwo3bqPtV4tR5tuPaUhTOo/kvhVwd8XwwOllGYkbg==" }, "safer-buffer": { "version": "2.1.2", @@ -4261,7 +4246,6 @@ "version": "1.0.2", "resolved": false, "integrity": "sha1-EYvfW4zcUaKn5w0hHgfisLmxB9M=", - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -4281,7 +4265,6 @@ "version": "3.0.1", "resolved": false, "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", - "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -4325,14 +4308,12 @@ "wrappy": { "version": "1.0.2", "resolved": false, - "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=", - "optional": true + "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=" }, "yallist": { "version": "3.0.2", "resolved": false, - "integrity": "sha1-hFK0u36Dx8GI2AQcGoN8dz1ti7k=", - "optional": true + "integrity": "sha1-hFK0u36Dx8GI2AQcGoN8dz1ti7k=" } } }, @@ -7526,7 +7507,6 @@ "resolved": "https://registry.npmjs.org/align-text/-/align-text-0.1.4.tgz", "integrity": "sha1-DNkKVhCT810KmSVsIrcGlDP60Rc=", "dev": true, - "optional": true, "requires": { "kind-of": "^3.0.2", "longest": "^1.0.1", @@ -7896,8 +7876,7 @@ "version": "1.1.6", "resolved": "https://registry.npmjs.org/is-buffer/-/is-buffer-1.1.6.tgz", "integrity": "sha512-NcdALwpXkTm5Zvvbk7owOUSvVvBKDgKP5/ewfXEznmQFfs4ZRmanOeKBTjRVjka3QFoN6XJ+9F3USqfHqTaU5w==", - "dev": true, - "optional": true + "dev": true }, "is-builtin-module": { "version": "1.0.0", @@ -7993,7 +7972,6 @@ "resolved": "https://registry.npmjs.org/kind-of/-/kind-of-3.2.2.tgz", "integrity": "sha1-MeohpzS6ubuw8yRm2JOupR5KPGQ=", "dev": true, - "optional": true, "requires": { "is-buffer": "^1.1.5" } @@ -8046,8 +8024,7 @@ "version": "1.0.1", "resolved": "https://registry.npmjs.org/longest/-/longest-1.0.1.tgz", "integrity": "sha1-MKCy2jj3N3DoKUoNIuZiXtd9AJc=", - "dev": true, - "optional": true + "dev": true }, "lru-cache": { "version": "4.1.3", @@ -8350,8 +8327,7 @@ "version": "1.6.1", "resolved": "https://registry.npmjs.org/repeat-string/-/repeat-string-1.6.1.tgz", "integrity": "sha1-jcrkcOHIirwtYA//Sndihtp15jc=", - "dev": true, - "optional": true + "dev": true }, "require-directory": { "version": "2.1.1", diff --git a/garden-service/src/commands/run/task.ts b/garden-service/src/commands/run/task.ts index 8e14bc0115..a098940d73 100644 --- a/garden-service/src/commands/run/task.ts +++ b/garden-service/src/commands/run/task.ts @@ -60,7 +60,7 @@ export class RunTaskCommand extends Command { await garden.actions.prepareEnvironment({ log }) const taskTask = await TaskTask.factory({ garden, graph, task, log, force: true, forceBuild: opts["force-build"] }) - const result = (await garden.processTasks([taskTask]))[taskTask.getBaseKey()] + const result = (await garden.processTasks([taskTask]))[taskTask.getKey()] if (!result.error) { log.info("") diff --git a/garden-service/src/logger/log-entry.ts b/garden-service/src/logger/log-entry.ts index 03aa003c4a..6be5be1d30 100644 --- a/garden-service/src/logger/log-entry.ts +++ b/garden-service/src/logger/log-entry.ts @@ -25,9 +25,9 @@ export interface LogEntryMetadata { task?: TaskMetadata } export interface TaskMetadata { type: string, - baseKey: string, + key: string, status: TaskLogStatus, - id: string, + uid: string, versionString: string, durationMs?: number, } diff --git a/garden-service/src/process.ts b/garden-service/src/process.ts index dd6fb3c9e0..6f6fae42d3 100644 --- a/garden-service/src/process.ts +++ b/garden-service/src/process.ts @@ -177,7 +177,7 @@ export async function processModules( await restartPromise return { - taskResults: {}, // TODO: Return latest results for each task baseKey processed between restarts? + taskResults: {}, // TODO: Return latest results for each task key processed between restarts? restartRequired: true, } diff --git a/garden-service/src/task-graph.ts b/garden-service/src/task-graph.ts index 1bfcfb0163..f65630f7e2 100644 --- a/garden-service/src/task-graph.ts +++ b/garden-service/src/task-graph.ts @@ -30,11 +30,11 @@ export interface TaskResult { } /** - * When multiple tasks with the same baseKey are completed during a call to processTasks, - * the result from the last processed is used (hence only one key-value pair here per baseKey). + * When multiple tasks with the same key are completed during a call to processTasks, + * the result from the last processed is used (hence only one key-value pair here per key). */ export interface TaskResults { - [baseKey: string]: TaskResult + [key: string]: TaskResult } export const DEFAULT_CONCURRENCY = 4 @@ -47,10 +47,10 @@ export class TaskGraph { private logEntryMap: LogEntryMap /** - * A given task instance (uniquely identified by its key) should always return the same - * list of dependencies (by baseKey) from its getDependencies method. + * A given task instance (uniquely identified by its id) should always return the same + * list of dependencies (by key) from its getDependencies method. */ - private taskDependencyCache: { [key: string]: Set } // sets of baseKeys + private taskDependencyCache: { [id: string]: Set } // sets of keys private resultCache: ResultCache private opQueue: PQueue @@ -82,8 +82,8 @@ export class TaskGraph { * the node's task's dependencies (from configuration). */ node.clear() - const taskDeps = this.taskDependencyCache[node.getKey()] || new Set() - node.setDependencies(taskNodes.filter(n => taskDeps.has(n.getBaseKey()))) + const taskDeps = this.taskDependencyCache[node.getId()] || new Set() + node.setDependencies(taskNodes.filter(n => taskDeps.has(n.getKey()))) } const newRootNodes = taskNodes.filter(n => n.getDependencies().length === 0) @@ -97,24 +97,24 @@ export class TaskGraph { if (this.index.getNode(task)) { this.garden.events.emit("taskPending", { addedAt: new Date(), - key: task.getBaseKey(), + key: task.getKey(), version: task.version, }) } } private getNode(task: BaseTask): TaskNode | null { + const id = task.getId() const key = task.getKey() - const baseKey = task.getBaseKey() const existing = this.index.getNodes() - .filter(n => n.getBaseKey() === baseKey && n.getKey() !== key) + .filter(n => n.getKey() === key && n.getId() !== id) .reverse()[0] if (existing) { - // A task with the same baseKey is already pending. + // A task with the same key is already pending. return existing } else { - const cachedResultExists = !!this.resultCache.get(task.getBaseKey(), task.version.versionString) + const cachedResultExists = !!this.resultCache.get(task.getKey(), task.version.versionString) if (cachedResultExists && !task.force) { // No need to add task or its dependencies. return null @@ -162,17 +162,17 @@ export class TaskGraph { return Bluebird.map(batch, async (node: TaskNode) => { const task = node.task const type = node.getType() - const baseKey = node.getBaseKey() + const key = node.getKey() const description = node.getDescription() - let result: TaskResult = { type, description, key: task.getBaseKey() } + let result: TaskResult = { type, description, key: task.getKey() } try { this.logTask(node) this.logEntryMap.inProgress.setState(inProgressToStr(this.inProgress.getNodes())) const dependencyBaseKeys = (await task.getDependencies()) - .map(dep => dep.getBaseKey()) + .map(dep => dep.getKey()) const dependencyResults = merge( this.resultCache.pick(dependencyBaseKeys), @@ -181,7 +181,7 @@ export class TaskGraph { try { this.garden.events.emit("taskProcessing", { startedAt: new Date(), - key: task.getBaseKey(), + key: task.getKey(), version: task.version, }) result = await node.process(dependencyResults) @@ -192,8 +192,8 @@ export class TaskGraph { this.logTaskError(node, error) this.cancelDependants(node) } finally { - results[baseKey] = result - this.resultCache.put(baseKey, task.version.versionString, result) + results[key] = result + this.resultCache.put(key, task.version.versionString, result) } } finally { this.completeTask(node, !result.error) @@ -223,7 +223,7 @@ export class TaskGraph { if (node) { const depTasks = await node.task.getDependencies() - this.taskDependencyCache[node.getKey()] = new Set(depTasks.map(d => d.getBaseKey())) + this.taskDependencyCache[node.getId()] = new Set(depTasks.map(d => d.getKey())) for (const dep of depTasks) { await this.addNodeWithDependencies(dep) } @@ -232,7 +232,7 @@ export class TaskGraph { private completeTask(node: TaskNode, success: boolean) { if (node.getDependencies().length > 0) { - throw new TaskGraphError(`Task ${node.getKey()} still has unprocessed dependencies`) + throw new TaskGraphError(`Task ${node.getId()} still has unprocessed dependencies`) } this.remove(node) @@ -256,7 +256,7 @@ export class TaskGraph { private getDependants(node: TaskNode): TaskNode[] { const dependants = this.index.getNodes().filter(n => n.getDependencies() - .find(d => d.getBaseKey() === node.getBaseKey())) + .find(d => d.getKey() === node.getKey())) return dependants.concat(flatten(dependants.map(d => this.getDependants(d)))) } @@ -264,25 +264,25 @@ export class TaskGraph { private logTask(node: TaskNode) { const entry = this.log.debug({ section: "tasks", - msg: `Processing task ${taskStyle(node.getKey())}`, + msg: `Processing task ${taskStyle(node.getId())}`, status: "active", metadata: metadataForLog(node.task, "active"), }) - this.logEntryMap[node.getKey()] = entry + this.logEntryMap[node.getId()] = entry } private logTaskComplete(node: TaskNode, success: boolean) { - const entry = this.logEntryMap[node.getKey()] + const entry = this.logEntryMap[node.getId()] if (entry) { - const keyStr = taskStyle(node.getKey()) + const idStr = taskStyle(node.getId()) if (success) { const durationSecs = entry.getDuration(3) const metadata = metadataForLog(node.task, "success") metadata.task!.durationMs = durationSecs * 1000 - entry.setSuccess({ msg: `Completed task ${keyStr} (took ${durationSecs} sec)`, metadata }) + entry.setSuccess({ msg: `Completed task ${idStr} (took ${durationSecs} sec)`, metadata }) } else { const metadata = metadataForLog(node.task, "error") - entry.setError({ msg: `Failed task ${keyStr}`, metadata }) + entry.setError({ msg: `Failed task ${idStr}`, metadata }) } } this.logEntryMap.counter.setState(remainingTasksToStr(this.index.length)) @@ -319,23 +319,23 @@ export class TaskGraph { } } -function getIndexKey(task: BaseTask) { - const key = task.getKey() +function getIndexId(task: BaseTask) { + const id = task.getId() - if (!task.type || !key || task.type.length === 0 || key.length === 0) { - throw new TaskDefinitionError("Tasks must define a type and a key") + if (!task.type || !id || task.type.length === 0 || id.length === 0) { + throw new TaskDefinitionError("Tasks must define a type and an id") } - return key + return id } function metadataForLog(task: BaseTask, status: TaskLogStatus): LogEntryMetadata { return { task: { type: task.type, - baseKey: task.getBaseKey(), + key: task.getKey(), status, - id: task.id, + uid: task.uid, versionString: task.version.versionString, }, } @@ -352,22 +352,22 @@ class TaskNodeMap { } getNode(task: BaseTask) { - const indexKey = getIndexKey(task) - const element = this.index.get(indexKey) + const taskId = getIndexId(task) + const element = this.index.get(taskId) return element } addNode(node: TaskNode): void { - const indexKey = node.getKey() + const taskId = node.getId() - if (!this.index.get(indexKey)) { - this.index.set(indexKey, node) + if (!this.index.get(taskId)) { + this.index.set(taskId, node) this.length++ } } removeNode(node: TaskNode): void { - if (this.index.delete(node.getKey())) { + if (this.index.delete(node.getId())) { this.length-- } } @@ -383,7 +383,7 @@ class TaskNodeMap { } contains(node: TaskNode): boolean { - return this.index.has(node.getKey()) + return this.index.has(node.getId()) } clear() { @@ -394,8 +394,8 @@ class TaskNodeMap { // For testing/debugging purposes inspect(): object { const out = {} - this.index.forEach((node, key) => { - out[key] = node.inspect() + this.index.forEach((node, id) => { + out[id] = node.inspect() }) return out } @@ -425,12 +425,12 @@ class TaskNode { return this.dependencies.getNodes() } - getBaseKey() { - return this.task.getBaseKey() + getKey() { + return this.task.getKey() } - getKey() { - return getIndexKey(this.task) + getId() { + return getIndexId(this.task) } getDescription() { @@ -444,7 +444,7 @@ class TaskNode { // For testing/debugging purposes inspect(): object { return { - key: this.getKey(), + id: this.getId(), dependencies: this.getDependencies().map(d => d.inspect()), } } @@ -454,7 +454,7 @@ class TaskNode { return { type: this.getType(), - key: this.getBaseKey(), + key: this.getKey(), description: this.getDescription(), output, dependencyResults, @@ -469,7 +469,7 @@ interface CachedResult { class ResultCache { /** - * By design, at most one TaskResult (the most recently processed) is cached for a given baseKey. + * By design, at most one TaskResult (the most recently processed) is cached for a given key. * * Invariant: No concurrent calls are made to this class' instance methods, since they * only happen within TaskGraph's addTaskInternal and processTasksInternal methods, @@ -482,28 +482,28 @@ class ResultCache { this.cache = {} } - put(baseKey: string, versionString: string, result: TaskResult): void { - this.cache[baseKey] = { result, versionString } + put(key: string, versionString: string, result: TaskResult): void { + this.cache[key] = { result, versionString } } - get(baseKey: string, versionString: string): TaskResult | null { - const r = this.cache[baseKey] + get(key: string, versionString: string): TaskResult | null { + const r = this.cache[key] return (r && r.versionString === versionString && !r.result.error) ? r.result : null } - getNewest(baseKey: string): TaskResult | null { - const r = this.cache[baseKey] + getNewest(key: string): TaskResult | null { + const r = this.cache[key] return (r && !r.result.error) ? r.result : null } - // Returns newest cached results, if any, for baseKeys - pick(baseKeys: string[]): TaskResults { + // Returns newest cached results, if any, for keys + pick(keys: string[]): TaskResults { const results: TaskResults = {} - for (const baseKey of baseKeys) { - const cachedResult = this.getNewest(baseKey) + for (const key of keys) { + const cachedResult = this.getNewest(key) if (cachedResult) { - results[baseKey] = cachedResult + results[key] = cachedResult } } @@ -517,7 +517,7 @@ interface LogEntryMap { [key: string]: LogEntry } const taskStyle = chalk.cyan.bold function inProgressToStr(nodes) { - return `Currently in progress [${nodes.map(n => taskStyle(n.getKey())).join(", ")}]` + return `Currently in progress [${nodes.map(n => taskStyle(n.getId())).join(", ")}]` } function remainingTasksToStr(num) { diff --git a/garden-service/src/tasks/base.ts b/garden-service/src/tasks/base.ts index efba0deea8..a4a5e5dd28 100644 --- a/garden-service/src/tasks/base.ts +++ b/garden-service/src/tasks/base.ts @@ -33,7 +33,7 @@ export abstract class BaseTask { abstract depType: DependencyGraphNodeType garden: Garden log: LogEntry - id: string + uid: string force: boolean version: ModuleVersion @@ -42,7 +42,7 @@ export abstract class BaseTask { constructor(initArgs: TaskParams) { this.garden = initArgs.garden this.dependencies = [] - this.id = uuidv1() // uuidv1 is timestamp-based + this.uid = uuidv1() // uuidv1 is timestamp-based this.force = !!initArgs.force this.version = initArgs.version this.log = initArgs.log @@ -54,12 +54,12 @@ export abstract class BaseTask { protected abstract getName(): string - getBaseKey(): string { + getKey(): string { return makeBaseKey(this.type, this.getName()) } - getKey(): string { - return `${this.getBaseKey()}.${this.id}` + getId(): string { + return `${this.getKey()}.${this.uid}` } abstract getDescription(): string diff --git a/garden-service/src/tasks/helpers.ts b/garden-service/src/tasks/helpers.ts index 639ed6516e..ea9ac5c8f4 100644 --- a/garden-service/src/tasks/helpers.ts +++ b/garden-service/src/tasks/helpers.ts @@ -76,7 +76,7 @@ export async function getDependantTasksForModule( const outputTasks = [...pushTasks, ...deployTasks] log.silly(`getDependantTasksForModule called for module ${module.name}, returning the following tasks:`) - log.silly(` ${outputTasks.map(t => t.getBaseKey()).join(", ")}`) + log.silly(` ${outputTasks.map(t => t.getKey()).join(", ")}`) return outputTasks } diff --git a/garden-service/test/integ-helpers.ts b/garden-service/test/integ-helpers.ts index c22ad0c9d9..3f7fc4a575 100644 --- a/garden-service/test/integ-helpers.ts +++ b/garden-service/test/integ-helpers.ts @@ -68,7 +68,7 @@ export type TaskLogEntryResult = { } /** - * Searches a log entry array for entries pertaining to tasks with baseKey, optionally filtered the task status + * Searches a log entry array for entries pertaining to tasks with key, optionally filtered the task status * indicated by the log entry. * * An example use would be to search for occurrences of "build.some-module" with the "success" status in the log, @@ -76,14 +76,14 @@ export type TaskLogEntryResult = { * e.g. indicate that the module was rebuilt when one of its files changed during the execution of a GardenWatch * instance). */ -export function findTasks(entries: JsonLogEntry[], baseKey: string, status?: TaskLogStatus): TaskLogEntryResult[] { +export function findTasks(entries: JsonLogEntry[], key: string, status?: TaskLogStatus): TaskLogEntryResult[] { - const matching: FilteredTasks = filterTasks(entries, baseKey, status) + const matching: FilteredTasks = filterTasks(entries, key, status) const taskIds: string[] = [] // List of task ids, ordered by their first appearance in the log. for (const match of matching) { - const taskId = match.entry.metadata!.task!.id + const taskId = match.entry.metadata!.task!.uid if (!taskIds.find(id => id === taskId)) { taskIds.push(taskId) } @@ -91,7 +91,7 @@ export function findTasks(entries: JsonLogEntry[], baseKey: string, status?: Tas return taskIds.map((taskId) => { - const matchesForKey = matching.filter(m => m.entry.metadata!.task!.id === taskId) + const matchesForKey = matching.filter(m => m.entry.metadata!.task!.uid === taskId) const startedMatch = matchesForKey.find(m => m.entry.metadata!.task!.status === "active") const errorMatch = matchesForKey.find(m => m.entry.metadata!.task!.status === "error") @@ -111,17 +111,17 @@ export function findTasks(entries: JsonLogEntry[], baseKey: string, status?: Tas /** * Returns the index of the matching log entry (in entries), or null if no matching entry was found. */ -export function findTask(entries: JsonLogEntry[], baseKey: string, status?: TaskLogStatus): number | null { - const index = entries.findIndex(e => matchTask(e, baseKey, status)) +export function findTask(entries: JsonLogEntry[], key: string, status?: TaskLogStatus): number | null { + const index = entries.findIndex(e => matchTask(e, key, status)) return index === -1 ? null : index } export type FilteredTasks = { entry: JsonLogEntry, index: number }[] -export function filterTasks(entries: JsonLogEntry[], baseKey: string, status?: TaskLogStatus): FilteredTasks { +export function filterTasks(entries: JsonLogEntry[], key: string, status?: TaskLogStatus): FilteredTasks { const filtered: FilteredTasks = [] for (const [index, entry] of entries.entries()) { - if (matchTask(entry, baseKey, status)) { + if (matchTask(entry, key, status)) { filtered.push({ index, entry }) } } @@ -129,7 +129,7 @@ export function filterTasks(entries: JsonLogEntry[], baseKey: string, status?: T return filtered } -export function matchTask(entry: JsonLogEntry, baseKey: string, status?: TaskLogStatus): boolean { +export function matchTask(entry: JsonLogEntry, key: string, status?: TaskLogStatus): boolean { const meta = get(entry, ["metadata", "task"]) - return !!meta && meta.baseKey === baseKey && (!status || status === meta.status) + return !!meta && meta.key === key && (!status || status === meta.status) } diff --git a/garden-service/test/integ/src/integ-helpers.ts b/garden-service/test/integ/src/integ-helpers.ts index ea00b97c6e..21e453b72e 100644 --- a/garden-service/test/integ/src/integ-helpers.ts +++ b/garden-service/test/integ/src/integ-helpers.ts @@ -18,15 +18,15 @@ describe("integ-helpers", () => { }) const specs = [ - { taskType: "build", baseKey: "build.result" }, - { taskType: "deploy", baseKey: "deploy.redis" }, - { taskType: "test", baseKey: "test.api.unit" }, + { taskType: "build", key: "build.result" }, + { taskType: "deploy", key: "deploy.redis" }, + { taskType: "test", key: "test.api.unit" }, ] - for (const { taskType, baseKey } of specs) { + for (const { taskType, key } of specs) { it(`should find a ${taskType} task`, () => { - const found = findTasks(testEntries, baseKey)[0] - expect(found, "entries not found").to.be.ok + const found = findTasks(testEntries, key)[0] + expect(found, `entries for ${key} not found`).to.be.ok const { startedIndex, completedIndex, executionTimeMs } = found expect([startedIndex, completedIndex, executionTimeMs]).to.not.include([null, undefined]) }) @@ -39,8 +39,8 @@ describe("integ-helpers", () => { it("should run and produce the expected output for a test command in the vote example project", async () => { const logEntries = await runGarden(voteExamplePath, ["test"]) expect(logEntries.length).to.be.greaterThan(0) - const found = findTasks(logEntries, "build.result")[0] - expect(found, "entries not found").to.be.ok + const found = findTasks(logEntries, "test.api.unit")[0] + expect(found, "entries for not test.api.unit found").to.be.ok const { startedIndex, completedIndex, executionTimeMs } = found expect([startedIndex, completedIndex, executionTimeMs]).to.not.include([null, undefined]) }) diff --git a/garden-service/test/integ/src/pre-release.ts b/garden-service/test/integ/src/pre-release.ts index 7dad7c37c4..5d844e44be 100644 --- a/garden-service/test/integ/src/pre-release.ts +++ b/garden-service/test/integ/src/pre-release.ts @@ -64,6 +64,10 @@ describe("PreReleaseTests", () => { await gardenWatch.run({ testSteps }) }) + after(async () => { + await deleteExampleNamespaces(log, false) + }) + }) describe("tasks", () => { @@ -84,6 +88,10 @@ describe("PreReleaseTests", () => { .to.eql("passed") }) + after(async () => { + await deleteExampleNamespaces(log, false) + }) + }) /* @@ -124,6 +132,10 @@ describe("PreReleaseTests", () => { }) + after(async () => { + await deleteExampleNamespaces(log, false) + }) + }) describe("vote-helm: helm & dependency calculations", () => { @@ -145,6 +157,10 @@ describe("PreReleaseTests", () => { }) + after(async () => { + await deleteExampleNamespaces(log, false) + }) + }) describe("remote sources", () => { @@ -162,4 +178,8 @@ describe("PreReleaseTests", () => { }) }) + after(async () => { + await deleteExampleNamespaces(log, false) + }) + }) diff --git a/garden-service/test/run-garden.ts b/garden-service/test/run-garden.ts index 976620c10d..2db74b735a 100644 --- a/garden-service/test/run-garden.ts +++ b/garden-service/test/run-garden.ts @@ -24,11 +24,11 @@ export function dashboardUpStep(): WatchTestStep { } } -export function taskCompletedStep(baseKey: string, completedCount: number, description?: string): WatchTestStep { +export function taskCompletedStep(key: string, completedCount: number, description?: string): WatchTestStep { return { - description: description || baseKey, + description: description || key, condition: async (logEntries: JsonLogEntry[]) => { - const tasks = findTasks(logEntries, baseKey) + const tasks = findTasks(logEntries, key) if (tasks.filter(t => t.completedIndex).length === completedCount) { return "passed" } @@ -78,6 +78,9 @@ export function commandReloadedStep(): WatchTestStep { */ export async function runGarden(dir: string, command: string[]): Promise { const out = (await execa(gardenBinPath, [...command, "--logger-type", "json", "-l", "4"], { cwd: dir })).stdout + if (showLog) { + console.log(out) + } return parseLogEntries(out.split("\n").filter(Boolean)) } diff --git a/garden-service/test/unit/src/task-graph.ts b/garden-service/test/unit/src/task-graph.ts index 9956aa81b1..ff0340a989 100644 --- a/garden-service/test/unit/src/task-graph.ts +++ b/garden-service/test/unit/src/task-graph.ts @@ -17,7 +17,7 @@ type TestTaskCallback = (name: string, result: any) => Promise interface TestTaskOptions { callback?: TestTaskCallback dependencies?: BaseTask[], - id?: string + uid?: string throwError?: boolean } @@ -26,7 +26,7 @@ class TestTask extends BaseTask { depType: DependencyGraphNodeType = "test" name: string callback: TestTaskCallback | null - id: string + uid: string throwError: boolean constructor( @@ -52,7 +52,7 @@ class TestTask extends BaseTask { this.name = name this.callback = options.callback || null - this.id = options.id || "" + this.uid = options.uid || "" this.throwError = !!options.throwError this.dependencies = options.dependencies || [] } @@ -61,23 +61,23 @@ class TestTask extends BaseTask { return this.name } - getBaseKey(): string { + getKey(): string { return this.name } - getKey(): string { - return this.id ? `${this.name}.${this.id}` : this.name + getId(): string { + return this.uid ? `${this.name}.${this.uid}` : this.name } getDescription() { - return this.getKey() + return this.getId() } async process(dependencyResults: TaskResults) { - const result = { result: "result-" + this.getKey(), dependencyResults } + const result = { result: "result-" + this.getId(), dependencyResults } if (this.callback) { - await this.callback(this.getKey(), result.result) + await this.callback(this.getId(), result.result) } if (this.throwError) { @@ -128,9 +128,9 @@ describe("task-graph", () => { const result = await graph.process([task]) expect(garden.events.eventLog).to.eql([ - { name: "taskPending", payload: { addedAt: now, key: task.getBaseKey(), version: task.version } }, + { name: "taskPending", payload: { addedAt: now, key: task.getKey(), version: task.version } }, { name: "taskGraphProcessing", payload: { startedAt: now } }, - { name: "taskProcessing", payload: { startedAt: now, key: task.getBaseKey(), version: task.version } }, + { name: "taskProcessing", payload: { startedAt: now, key: task.getKey(), version: task.version } }, { name: "taskComplete", payload: result["a"] }, { name: "taskGraphComplete", payload: { completedAt: now } }, ]) @@ -147,7 +147,7 @@ describe("task-graph", () => { garden.events.eventLog = [] - // repeatedTask has the same baseKey and version as task, so its result is already cached + // repeatedTask has the same key and version as task, so its result is already cached const repeatedTask = new TestTask(garden, "a", false) await graph.process([repeatedTask]) @@ -167,9 +167,9 @@ describe("task-graph", () => { const result = await graph.process([task]) expect(garden.events.eventLog).to.eql([ - { name: "taskPending", payload: { addedAt: now, key: task.getBaseKey(), version: task.version } }, + { name: "taskPending", payload: { addedAt: now, key: task.getKey(), version: task.version } }, { name: "taskGraphProcessing", payload: { startedAt: now } }, - { name: "taskProcessing", payload: { startedAt: now, key: task.getBaseKey(), version: task.version } }, + { name: "taskProcessing", payload: { startedAt: now, key: task.getKey(), version: task.version } }, { name: "taskError", payload: result["a"] }, { name: "taskGraphComplete", payload: { completedAt: now } }, ]) @@ -189,10 +189,10 @@ describe("task-graph", () => { const opts = { callback } - const taskA = new TestTask(garden, "a", false, { ...opts, dependencies: [], id: "a1" }) - const taskB = new TestTask(garden, "b", false, { ...opts, dependencies: [taskA], id: "b1" }) - const taskC = new TestTask(garden, "c", false, { ...opts, dependencies: [taskB], id: "c1" }) - const taskD = new TestTask(garden, "d", false, { ...opts, dependencies: [taskB, taskC], id: "d1" }) + const taskA = new TestTask(garden, "a", false, { ...opts, dependencies: [], uid: "a1" }) + const taskB = new TestTask(garden, "b", false, { ...opts, dependencies: [taskA], uid: "b1" }) + const taskC = new TestTask(garden, "c", false, { ...opts, dependencies: [taskB], uid: "c1" }) + const taskD = new TestTask(garden, "d", false, { ...opts, dependencies: [taskB, taskC], uid: "d1" }) // we should be able to add tasks multiple times and in any order const results = await graph.process([ @@ -220,13 +220,13 @@ describe("task-graph", () => { const repeatOpts = { callback: repeatCallback } - const repeatTaskA = new TestTask(garden, "a", false, { ...repeatOpts, dependencies: [], id: "a2" }) - const repeatTaskB = new TestTask(garden, "b", false, { ...repeatOpts, dependencies: [repeatTaskA], id: "b2" }) - const repeatTaskC = new TestTask(garden, "c", true, { ...repeatOpts, dependencies: [repeatTaskB], id: "c2" }) + const repeatTaskA = new TestTask(garden, "a", false, { ...repeatOpts, dependencies: [], uid: "a2" }) + const repeatTaskB = new TestTask(garden, "b", false, { ...repeatOpts, dependencies: [repeatTaskA], uid: "b2" }) + const repeatTaskC = new TestTask(garden, "c", true, { ...repeatOpts, dependencies: [repeatTaskB], uid: "c2" }) - const repeatTaskAforced = new TestTask(garden, "a", true, { ...repeatOpts, dependencies: [], id: "a2f" }) + const repeatTaskAforced = new TestTask(garden, "a", true, { ...repeatOpts, dependencies: [], uid: "a2f" }) const repeatTaskBforced = new TestTask(garden, "b", true, - { ...repeatOpts, dependencies: [repeatTaskA], id: "b2f" }) + { ...repeatOpts, dependencies: [repeatTaskA], uid: "b2f" }) await graph.process([ repeatTaskBforced, diff --git a/garden-service/test/unit/src/tasks/helpers.ts b/garden-service/test/unit/src/tasks/helpers.ts index 7846f301d2..9f97904a55 100644 --- a/garden-service/test/unit/src/tasks/helpers.ts +++ b/garden-service/test/unit/src/tasks/helpers.ts @@ -16,7 +16,7 @@ async function sortedBaseKeysdependencyTasks(tasks: BaseTask[]): Promise t.getBaseKey())).sort() + return uniq(tasks.map(t => t.getKey())).sort() } describe("TaskHelpers", () => {