From 3db05a5aac5a1241db4035e4c5f288b15eca0db4 Mon Sep 17 00:00:00 2001 From: Elias Kassell Date: Fri, 21 Aug 2020 11:10:01 +0100 Subject: [PATCH] Always run auto assertions (#959) * Always run auto assertions * Change parent to be a target, tidied attachment * Add name fixing * Change assertion in prune to use target * Tidy IAction, rename to actionParent, bump version * Added missing no null check for parent * Add auto assertions to caching test --- api/commands/prune.ts | 9 +++++++++ core/session.ts | 5 +++++ core/table.ts | 28 ++++++++-------------------- protos/core.proto | 3 +++ tests/api/api.spec.ts | 21 +++++++++++++++++++-- tests/api/examples.spec.ts | 2 -- tests/integration/bigquery.spec.ts | 5 +++++ version.bzl | 2 +- 8 files changed, 50 insertions(+), 25 deletions(-) diff --git a/api/commands/prune.ts b/api/commands/prune.ts index 16e1b0d53..835a75b5f 100644 --- a/api/commands/prune.ts +++ b/api/commands/prune.ts @@ -77,5 +77,14 @@ function computeIncludedActionNames( } } + // Add auto assertions + [...compiledGraph.assertions].forEach(assertion => { + if (!!assertion.parentAction) { + if (includedActionNames.has(utils.targetToName(assertion.parentAction))) { + includedActionNames.add(utils.targetToName(assertion.target)); + } + } + }); + return includedActionNames; } diff --git a/core/session.ts b/core/session.ts index 30681a8fd..64992c0bb 100644 --- a/core/session.ts +++ b/core/session.ts @@ -39,6 +39,7 @@ export interface IActionProto { hermeticity?: dataform.ActionHermeticity; target?: dataform.ITarget; canonicalTarget?: dataform.ITarget; + parentAction?: dataform.ITarget; } type SqlxConfig = ( @@ -515,6 +516,10 @@ export class Session { action.dependencies = (action.dependencyTargets || []).map(dependencyTarget => utils.targetToName(dependencyTarget) ); + + if (!!action.parentAction) { + action.parentAction = newTargetByOriginalTarget.get(action.parentAction); + } }); } diff --git a/core/table.ts b/core/table.ts index 9b0488be1..3325d442a 100644 --- a/core/table.ts +++ b/core/table.ts @@ -299,8 +299,6 @@ export class Table { private contextableWhere: Contextable; private contextablePreOps: Array> = []; private contextablePostOps: Array> = []; - private uniqueKeyAssertion?: Assertion; - private mergedRowConditionsAssertion?: Assertion; public config(config: ITableConfig) { checkExcessProperties( @@ -450,12 +448,6 @@ export class Table { newTags.forEach(t => { this.proto.tags.push(t); }); - if (!!this.uniqueKeyAssertion) { - this.uniqueKeyAssertion.tags(value); - } - if (!!this.mergedRowConditionsAssertion) { - this.mergedRowConditionsAssertion.tags(value); - } return this; } @@ -510,11 +502,9 @@ export class Table { if (!!assertions.uniqueKey) { const indexCols = typeof assertions.uniqueKey === "string" ? [assertions.uniqueKey] : assertions.uniqueKey; - this.uniqueKeyAssertion = this.session - .assert(`${this.proto.target.name}_assertions_uniqueKey`, ctx => - this.session.adapter().indexAssertion(ctx.ref(this.proto.target), indexCols) - ) - .tags(this.proto.tags); + this.session.assert(`${this.proto.target.name}_assertions_uniqueKey`, ctx => + this.session.adapter().indexAssertion(ctx.ref(this.proto.target), indexCols) + ).proto.parentAction = this.proto.target; } const mergedRowConditions = assertions.rowConditions || []; if (!!assertions.nonNull) { @@ -523,13 +513,11 @@ export class Table { nonNullCols.forEach(nonNullCol => mergedRowConditions.push(`${nonNullCol} IS NOT NULL`)); } if (!!mergedRowConditions && mergedRowConditions.length > 0) { - this.mergedRowConditionsAssertion = this.session - .assert(`${this.proto.target.name}_assertions_rowConditions`, ctx => - this.session - .adapter() - .rowConditionsAssertion(ctx.ref(this.proto.target), mergedRowConditions) - ) - .tags(this.proto.tags); + this.session.assert(`${this.proto.target.name}_assertions_rowConditions`, ctx => + this.session + .adapter() + .rowConditionsAssertion(ctx.ref(this.proto.target), mergedRowConditions) + ).proto.parentAction = this.proto.target; } return this; } diff --git a/protos/core.proto b/protos/core.proto index 7cef01300..741e4f83d 100644 --- a/protos/core.proto +++ b/protos/core.proto @@ -335,6 +335,9 @@ message Assertion { repeated string tags = 9; ActionDescriptor action_descriptor = 10; + + // Only present for auto assertions. + Target parent_action = 15; // Generated. string file_name = 7; diff --git a/tests/api/api.spec.ts b/tests/api/api.spec.ts index 4cd5970e1..2c0d6f3a0 100644 --- a/tests/api/api.spec.ts +++ b/tests/api/api.spec.ts @@ -47,6 +47,19 @@ suite("@dataform/api", () => { }, query: "query" } + ], + assertions: [ + { + name: "schema.d", + target: { + schema: "schema", + name: "d" + }, + parentAction: { + schema: "schema", + name: "b" + } + } ] }); @@ -325,20 +338,24 @@ suite("@dataform/api", () => { const prunedGraph = prune(TEST_GRAPH, { actions: ["schema.a"], includeDependencies: true }); const actionNames = [ ...prunedGraph.tables.map(action => action.name), - ...prunedGraph.operations.map(action => action.name) + ...prunedGraph.operations.map(action => action.name), + ...prunedGraph.assertions.map(action => action.name) ]; expect(actionNames).includes("schema.a"); expect(actionNames).includes("schema.b"); + expect(actionNames).includes("schema.d"); }); test("prune actions with --actions without dependencies", () => { const prunedGraph = prune(TEST_GRAPH, { actions: ["schema.a"], includeDependencies: false }); const actionNames = [ ...prunedGraph.tables.map(action => action.name), - ...prunedGraph.operations.map(action => action.name) + ...prunedGraph.operations.map(action => action.name), + ...prunedGraph.assertions.map(action => action.name) ]; expect(actionNames).includes("schema.a"); expect(actionNames).not.includes("schema.b"); + expect(actionNames).not.includes("schema.d"); }); }); diff --git a/tests/api/examples.spec.ts b/tests/api/examples.spec.ts index 5dd675ce4..12a7d5874 100644 --- a/tests/api/examples.spec.ts +++ b/tests/api/examples.spec.ts @@ -322,7 +322,6 @@ suite("examples", () => { name: "example_table_with_tags" }) ]); - expect(exampleTableWithTagsUniqueKeyAssertion.tags).eql(["tag1", "tag2", "tag3"]); // Check table-with-tags's row conditions assertion const exampleTableWithTagsRowConditionsAssertion = graph.assertions.filter( @@ -347,7 +346,6 @@ suite("examples", () => { name: "example_table_with_tags" }) ]); - expect(exampleTableWithTagsRowConditionsAssertion.tags).eql(["tag1", "tag2", "tag3"]); // Check sample data const exampleSampleData = graph.tables.find( diff --git a/tests/integration/bigquery.spec.ts b/tests/integration/bigquery.spec.ts index a3d9f3093..02979e2f6 100644 --- a/tests/integration/bigquery.spec.ts +++ b/tests/integration/bigquery.spec.ts @@ -201,6 +201,11 @@ suite("@dataform/integration/bigquery", { parallel: true }, ({ before, after }) // Should run because the dataset has changed in the warehouse. "dataform-integration-tests.df_integration_test_run_caching.sample_data_2": dataform.ActionResult.ExecutionStatus.SUCCESSFUL, + // Should run because they are auto assertions. + "dataform-integration-tests.df_integration_test_assertions_run_caching.sample_data_2_assertions_uniqueKey": + dataform.ActionResult.ExecutionStatus.SUCCESSFUL, + "dataform-integration-tests.df_integration_test_assertions_run_caching.sample_data_2_assertions_rowConditions": + dataform.ActionResult.ExecutionStatus.SUCCESSFUL, // Should run because an input to dataset has changed in the warehouse. "dataform-integration-tests.df_integration_test_run_caching.depends_on_sample_data_3": dataform.ActionResult.ExecutionStatus.SUCCESSFUL, diff --git a/version.bzl b/version.bzl index a15ad5528..69a6495bd 100644 --- a/version.bzl +++ b/version.bzl @@ -1,3 +1,3 @@ # NOTE: If you change the format of this line, you must change the bash command # in /scripts/publish to extract the version string correctly. -DF_VERSION = "1.9.0" +DF_VERSION = "1.9.1"