Skip to content

Commit

Permalink
fix(core): include deps in test version
Browse files Browse the repository at this point in the history 
This ensures that tests are re-run when a downstream dependency changes
that is more than one hop away in the config graph.
  • Loading branch information
@thsig @eysi09
thsig authored and eysi09 committed Apr 25, 2021
1 parent 01ba432 commit 8e7ce4c
Show file tree
Hide file tree
Showing 18 changed files with 162 additions and 89 deletions.
2 changes: 1 addition & 1 deletion core/src/commands/get/get-status.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ async function getTestStatuses(garden: Garden, configGraph: ConfigGraph, log: Lo
const result = await actions.getTestResult({
module,
log,
test: testFromConfig(module, testConfig),
test: testFromConfig(module, testConfig, configGraph),
})
return [`${module.name}.${testConfig.name}`, runStatus(result)]
})
Expand Down
2 changes: 1 addition & 1 deletion core/src/commands/get/get-test-result.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class GetTestResultCommand extends Command<Args> {
const actions = await garden.getActionRouter()

const module = graph.getModule(moduleName)
const test = testFromModule(module, testName)
const test = testFromModule(module, testName, graph)

const testResult = await actions.getTestResult({
log,
Expand Down
2 changes: 1 addition & 1 deletion core/src/commands/run/test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export class RunTestCommand extends Command<Args, Opts> {
})
}

const test = testFromConfig(module, testConfig)
const test = testFromConfig(module, testConfig, graph)

if ((module.disabled || test.disabled) && !opts.force) {
throw new CommandError(
Expand Down
2 changes: 1 addition & 1 deletion core/src/config-graph.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ export class ConfigGraph {
*/
getTest(moduleName: string, testName: string, includeDisabled?: boolean): GardenTest {
const module = this.getModule(moduleName, includeDisabled)
return testFromModule(module, testName)
return testFromModule(module, testName, this)
}

/*
Expand Down
2 changes: 1 addition & 1 deletion core/src/tasks/test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ export async function getTestTasks({
log,
force,
forceBuild,
test: testFromConfig(module, testConfig),
test: testFromConfig(module, testConfig, graph),
hotReloadServiceNames,
})
)
Expand Down
34 changes: 29 additions & 5 deletions core/src/types/test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@

import { GardenModule } from "./module"
import { TestConfig, testConfigSchema } from "../config/test"
import { getEntityVersion } from "../vcs/vcs"
import { getEntityVersion, hashStrings, versionStringPrefix } from "../vcs/vcs"
import { findByName } from "../util/util"
import { NotFoundError } from "../exceptions"
import { joi, joiUserIdentifier, versionStringSchema } from "../config/common"
import { ConfigGraph } from "../config-graph"
import { makeTestTaskName } from "../tasks/helpers"
import { sortBy } from "lodash"
import { serializeConfig } from "../config/module"

export interface GardenTest<M extends GardenModule = GardenModule> {
name: string
Expand All @@ -35,23 +39,43 @@ export const testSchema = () =>
version: versionStringSchema().description("The version of the test."),
})

export function testFromConfig<M extends GardenModule = GardenModule>(module: M, config: TestConfig): GardenTest<M> {
export function testFromConfig<M extends GardenModule = GardenModule>(
module: M,
config: TestConfig,
graph: ConfigGraph
): GardenTest<M> {
const deps = graph.getDependencies({
nodeType: "test",
name: makeTestTaskName(module.name, config.name),
recursive: true,
})
// We sort the dependencies by type and name to avoid unnecessary cache invalidation due to possible ordering changes.
const depHashes = [
...sortBy(deps.build, (mod) => mod.name).map((mod) => mod.version),
...sortBy(deps.deploy, (s) => s.module.name).map((s) => serializeConfig(s)),
...sortBy(deps.run, (t) => t.module.name).map((t) => serializeConfig(t)),
]
const version = `${versionStringPrefix}${hashStrings([getEntityVersion(module, config), ...depHashes])}`
return {
name: config.name,
module,
disabled: module.disabled || config.disabled,
config,
spec: config.spec,
version: getEntityVersion(module, config),
version,
}
}

export function testFromModule<M extends GardenModule = GardenModule>(module: M, name: string): GardenTest<M> {
export function testFromModule<M extends GardenModule = GardenModule>(
module: M,
name: string,
graph: ConfigGraph
): GardenTest<M> {
const config = findByName(module.testConfigs, name)

if (!config) {
throw new NotFoundError(`Could not find test ${name} in module ${module.name}`, { module, name })
}

return testFromConfig(module, config)
return testFromConfig(module, config, graph)
}
2 changes: 1 addition & 1 deletion core/src/vcs/vcs.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ export function getEntityVersion(module: GardenModule, entityConfig: ServiceConf
return `${versionStringPrefix}${hashStrings([module.version.versionString, configString])}`
}

function hashStrings(hashes: string[]) {
export function hashStrings(hashes: string[]) {
const versionHash = createHash("sha256")
versionHash.update(hashes.join("."))
return versionHash.digest("hex").slice(0, 10)
Expand Down
12 changes: 6 additions & 6 deletions core/test/integ/src/plugins/hadolint/hadolint.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ describe("hadolint provider", () => {
garden,
log: garden.log,
graph,
test: testFromConfig(module, module.testConfigs[0]),
test: testFromConfig(module, module.testConfigs[0], graph),
force: true,
forceBuild: false,
})
Expand Down Expand Up @@ -248,7 +248,7 @@ describe("hadolint provider", () => {
garden,
log: garden.log,
graph,
test: testFromConfig(module, module.testConfigs[0]),
test: testFromConfig(module, module.testConfigs[0], graph),
force: true,
forceBuild: false,
})
Expand Down Expand Up @@ -305,7 +305,7 @@ describe("hadolint provider", () => {
garden,
log: garden.log,
graph,
test: testFromConfig(module, module.testConfigs[0]),
test: testFromConfig(module, module.testConfigs[0], graph),
force: true,
forceBuild: false,
})
Expand Down Expand Up @@ -356,7 +356,7 @@ describe("hadolint provider", () => {
garden,
log: garden.log,
graph,
test: testFromConfig(module, module.testConfigs[0]),
test: testFromConfig(module, module.testConfigs[0], graph),
force: true,
forceBuild: false,
})
Expand Down Expand Up @@ -397,7 +397,7 @@ describe("hadolint provider", () => {
garden,
log: garden.log,
graph,
test: testFromConfig(module, module.testConfigs[0]),
test: testFromConfig(module, module.testConfigs[0], graph),
force: true,
forceBuild: false,
})
Expand Down Expand Up @@ -439,7 +439,7 @@ describe("hadolint provider", () => {

const testTask = new TestTask({
garden,
test: testFromConfig(module, module.testConfigs[0]),
test: testFromConfig(module, module.testConfigs[0], graph),
log: garden.log,
graph,
force: true,
Expand Down
14 changes: 7 additions & 7 deletions core/test/integ/src/plugins/kubernetes/container/container.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ describe("kubernetes container module handlers", () => {
const testTask = new TestTask({
garden,
graph,
test: testFromModule(module, "echo-test"),
test: testFromModule(module, "echo-test", graph),
log: garden.log,
force: true,
forceBuild: false,
Expand All @@ -203,7 +203,7 @@ describe("kubernetes container module handlers", () => {
const testConfig = findByName(module.testConfigs, "echo-test")!
testConfig.spec.command = ["bork"] // this will fail

const test = testFromConfig(module, testConfig)
const test = testFromConfig(module, testConfig, graph)

const testTask = new TestTask({
garden,
Expand Down Expand Up @@ -240,7 +240,7 @@ describe("kubernetes container module handlers", () => {
const testTask = new TestTask({
garden,
graph,
test: testFromModule(module, "artifacts-test"),
test: testFromModule(module, "artifacts-test", graph),
log: garden.log,
force: true,
forceBuild: false,
Expand All @@ -260,7 +260,7 @@ describe("kubernetes container module handlers", () => {
const testTask = new TestTask({
garden,
graph,
test: testFromModule(module, "artifacts-test-fail"),
test: testFromModule(module, "artifacts-test-fail", graph),
log: garden.log,
force: true,
forceBuild: false,
Expand All @@ -282,7 +282,7 @@ describe("kubernetes container module handlers", () => {
const testTask = new TestTask({
garden,
graph,
test: testFromModule(module, "globs-test"),
test: testFromModule(module, "globs-test", graph),
log: garden.log,
force: true,
forceBuild: false,
Expand All @@ -302,7 +302,7 @@ describe("kubernetes container module handlers", () => {
const testTask = new TestTask({
garden,
graph,
test: testFromConfig(module, module.testConfigs[0]),
test: testFromConfig(module, module.testConfigs[0], graph),
log: garden.log,
force: true,
forceBuild: false,
Expand All @@ -326,7 +326,7 @@ describe("kubernetes container module handlers", () => {
const testTask = new TestTask({
garden,
graph,
test: testFromConfig(module, module.testConfigs[0]),
test: testFromConfig(module, module.testConfigs[0], graph),
log: garden.log,
force: true,
forceBuild: false,
Expand Down
12 changes: 6 additions & 6 deletions core/test/integ/src/plugins/kubernetes/helm/test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe("testHelmModule", () => {
const testTask = new TestTask({
garden,
graph,
test: testFromModule(module, "echo-test"),
test: testFromModule(module, "echo-test", graph),
log: garden.log,
force: true,
forceBuild: false,
Expand All @@ -55,7 +55,7 @@ describe("testHelmModule", () => {
const testTask = new TestTask({
garden,
graph,
test: testFromModule(module, "echo-test"),
test: testFromModule(module, "echo-test", graph),
log: garden.log,
force: true,
forceBuild: false,
Expand All @@ -75,7 +75,7 @@ describe("testHelmModule", () => {
const testConfig = findByName(module.testConfigs, "echo-test")!
testConfig.spec.command = ["bork"] // this will fail

const test = testFromConfig(module, testConfig)
const test = testFromConfig(module, testConfig, graph)

const testTask = new TestTask({
garden,
Expand Down Expand Up @@ -110,7 +110,7 @@ describe("testHelmModule", () => {
const testTask = new TestTask({
garden,
graph,
test: testFromModule(module, "artifacts-test"),
test: testFromModule(module, "artifacts-test", graph),
log: garden.log,
force: true,
forceBuild: false,
Expand All @@ -130,7 +130,7 @@ describe("testHelmModule", () => {
const testTask = new TestTask({
garden,
graph,
test: testFromModule(module, "artifacts-test-fail"),
test: testFromModule(module, "artifacts-test-fail", graph),
log: garden.log,
force: true,
forceBuild: false,
Expand All @@ -152,7 +152,7 @@ describe("testHelmModule", () => {
const testTask = new TestTask({
garden,
graph,
test: testFromModule(module, "globs-test"),
test: testFromModule(module, "globs-test", graph),
log: garden.log,
force: true,
forceBuild: false,
Expand Down
12 changes: 6 additions & 6 deletions core/test/integ/src/plugins/kubernetes/kubernetes-module/test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe("testKubernetesModule", () => {
const testTask = new TestTask({
garden,
graph,
test: testFromModule(module, "echo-test"),
test: testFromModule(module, "echo-test", graph),
log: garden.log,
force: true,
forceBuild: false,
Expand All @@ -55,7 +55,7 @@ describe("testKubernetesModule", () => {
const testTask = new TestTask({
garden,
graph,
test: testFromModule(module, "with-namespace-test"),
test: testFromModule(module, "with-namespace-test", graph),
log: garden.log,
force: true,
forceBuild: false,
Expand All @@ -75,7 +75,7 @@ describe("testKubernetesModule", () => {
const testConfig = findByName(module.testConfigs, "echo-test")!
testConfig.spec.command = ["bork"] // this will fail

const test = testFromConfig(module, testConfig)
const test = testFromConfig(module, testConfig, graph)

const testTask = new TestTask({
garden,
Expand Down Expand Up @@ -110,7 +110,7 @@ describe("testKubernetesModule", () => {
const testTask = new TestTask({
garden,
graph,
test: testFromModule(module, "artifacts-test"),
test: testFromModule(module, "artifacts-test", graph),
log: garden.log,
force: true,
forceBuild: false,
Expand All @@ -130,7 +130,7 @@ describe("testKubernetesModule", () => {
const testTask = new TestTask({
garden,
graph,
test: testFromModule(module, "artifacts-test-fail"),
test: testFromModule(module, "artifacts-test-fail", graph),
log: garden.log,
force: true,
forceBuild: false,
Expand All @@ -152,7 +152,7 @@ describe("testKubernetesModule", () => {
const testTask = new TestTask({
garden,
graph,
test: testFromModule(module, "globs-test"),
test: testFromModule(module, "globs-test", graph),
log: garden.log,
force: true,
forceBuild: false,
Expand Down
2 changes: 1 addition & 1 deletion core/test/integ/src/plugins/kubernetes/system.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe("System services", () => {
const modules = graph.getModules().filter((module) => module.name.startsWith("conftest-"))

await Bluebird.map(modules, async (module) => {
const test = testFromConfig(module, module.testConfigs[0])
const test = testFromConfig(module, module.testConfigs[0], graph)
const testTask = new TestTask({
garden: systemGarden,
test,
Expand Down
Loading

0 comments on commit 8e7ce4c

Please sign in to comment.