Skip to content

Commit

Permalink
fix(core): make test task deps hot-reload aware
Browse files Browse the repository at this point in the history
Before this fix, the getDependencies method of TestTask returned
deploy tasks without the hot reload flag set appropriately.

This meant that repeated hot reloads of services with test dependants
would fail (because the test dependant would redeploy the service
without hot reloading enabled) when using the dev command.
  • Loading branch information
thsig committed Jan 28, 2020
1 parent 469453e commit 4af27b1
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 5 deletions.
1 change: 1 addition & 0 deletions garden-service/src/commands/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ export class DevCommand extends Command<DevCommandArgs, DevCommandOpts> {
module: m,
graph: updatedGraph,
filterNames,
hotReloadServiceNames,
})
)
)
Expand Down
20 changes: 16 additions & 4 deletions garden-service/src/tasks/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import Bluebird from "bluebird"
import chalk from "chalk"
import { find } from "lodash"
import { find, includes } from "lodash"
import minimatch = require("minimatch")

import { Module } from "../types/module"
Expand Down Expand Up @@ -40,6 +40,7 @@ export interface TestTaskParams {
testConfig: TestConfig
force: boolean
forceBuild: boolean
hotReloadServiceNames?: string[]
}

export class TestTask extends BaseTask {
Expand All @@ -49,6 +50,7 @@ export class TestTask extends BaseTask {
private graph: ConfigGraph
private testConfig: TestConfig
private forceBuild: boolean
private hotReloadServiceNames: string[]

constructor({
garden,
Expand All @@ -59,6 +61,7 @@ export class TestTask extends BaseTask {
force,
forceBuild,
version,
hotReloadServiceNames = [],
}: TestTaskParams & TaskParams & { _guard: true }) {
// Note: The _guard attribute is to prevent accidentally bypassing the factory method
super({ garden, log, force, version })
Expand All @@ -67,6 +70,7 @@ export class TestTask extends BaseTask {
this.testConfig = testConfig
this.force = force
this.forceBuild = forceBuild
this.hotReloadServiceNames = hotReloadServiceNames
}

static async factory(initArgs: TestTaskParams): Promise<TestTask> {
Expand All @@ -83,7 +87,12 @@ export class TestTask extends BaseTask {
}

const dg = this.graph
const deps = await dg.getDependencies({ nodeType: "test", name: this.getName(), recursive: false })
const deps = await dg.getDependencies({
nodeType: "test",
name: this.getName(),
recursive: false,
filterFn: (depNode) => !(depNode.type === "deploy" && includes(this.hotReloadServiceNames, depNode.name)),
})

const buildTasks = await BuildTask.factory({
garden: this.garden,
Expand All @@ -103,7 +112,7 @@ export class TestTask extends BaseTask {
})
})

const serviceTasks = deps.deploy.map(
const deployTasks = deps.deploy.map(
(service) =>
new DeployTask({
garden: this.garden,
Expand All @@ -115,7 +124,7 @@ export class TestTask extends BaseTask {
})
)

return [...buildTasks, ...serviceTasks, ...taskTasks]
return [...buildTasks, ...deployTasks, ...taskTasks]
}

getName() {
Expand Down Expand Up @@ -220,6 +229,7 @@ export async function getTestTasks({
graph,
module,
filterNames,
hotReloadServiceNames,
force = false,
forceBuild = false,
}: {
Expand All @@ -228,6 +238,7 @@ export async function getTestTasks({
graph: ConfigGraph
module: Module
filterNames?: string[]
hotReloadServiceNames?: string[]
force?: boolean
forceBuild?: boolean
}) {
Expand All @@ -248,6 +259,7 @@ export async function getTestTasks({
forceBuild,
testConfig: test,
module,
hotReloadServiceNames,
})
)
}
Expand Down
22 changes: 21 additions & 1 deletion garden-service/test/unit/src/tasks/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import { expect } from "chai"
import { resolve } from "path"
import { TestTask } from "../../../../src/tasks/test"
import { TestTask, getTestTasks } from "../../../../src/tasks/test"
import td from "testdouble"
import { Garden } from "../../../../src/garden"
import { dataDir, makeTestGarden } from "../../../helpers"
Expand Down Expand Up @@ -96,4 +96,24 @@ describe("TestTask", () => {
expect(deps.map((d) => d.getKey())).to.eql(["build.module-a", "deploy.service-b", "task.task-a"])
})
})

describe("getTestTasks", () => {
it("should not return test tasks with deploy dependencies on services deployed with hot reloading", async () => {
const moduleA = await graph.getModule("module-a")

const tasks = await getTestTasks({
garden,
log,
graph,
module: moduleA,
hotReloadServiceNames: ["service-b"],
})

const testTask = tasks[0]
const deps = await testTask.getDependencies()

expect(tasks.length).to.eql(1)
expect(deps.map((d) => d.getKey())).to.eql(["build.module-a", "task.task-a"])
})
})
})

0 comments on commit 4af27b1

Please sign in to comment.