Skip to content

Commit

Permalink
fix: don't emit taskPending if task is skipped
Browse files Browse the repository at this point in the history
This fixes an issue in TaskGraph where taskPending events were emitted
for tasks with up-to-date cached results. This led to the task graph UI
on the dashboard displaying the nodes corresponding to skipped tasks in
a permanent pending state.

This was addressed by only emitting the event if the task was, in fact,
added to TaskGraph's index.
  • Loading branch information
thsig committed Feb 21, 2019
1 parent 9e71353 commit d9196b2
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 5 deletions.
12 changes: 7 additions & 5 deletions garden-service/src/task-graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,15 @@ export class TaskGraph {
}

private async addTaskInternal(task: BaseTask) {
this.garden.events.emit("taskPending", {
addedAt: new Date(),
key: task.getKey(),
version: task.version,
})
await this.addNodeWithDependencies(task)
await this.rebuild()
if (this.index.getNode(task)) {
this.garden.events.emit("taskPending", {
addedAt: new Date(),
key: task.getKey(),
version: task.version,
})
}
}

private getNode(task: BaseTask): TaskNode | null {
Expand Down
22 changes: 22 additions & 0 deletions garden-service/test/src/task-graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,28 @@ describe("task-graph", () => {
])
})

it.only("should not emit a taskPending event when adding a task with a cached result", async () => {
const now = freezeTime()

const garden = await getGarden()
const graph = new TaskGraph(garden, garden.log)

const task = new TestTask(garden, "a", false)
await graph.addTask(task)
const result = await graph.processTasks()

// repeatedTask has the same baseKey and version as task, so its result is already cached
const repeatedTask = new TestTask(garden, "a", false)
await graph.addTask(repeatedTask)

expect(garden.events.log).to.eql([
{ name: "taskPending", payload: { addedAt: now, key: task.getKey(), version: task.version } },
{ name: "taskGraphProcessing", payload: { startedAt: now } },
{ name: "taskComplete", payload: result["a"] },
{ name: "taskGraphComplete", payload: { completedAt: now } },
])
})

it("should emit events when processing and completing a task", async () => {
const now = freezeTime()

Expand Down

0 comments on commit d9196b2

Please sign in to comment.