diff --git a/core/actions/index.ts b/core/actions/index.ts index ac69ae87b..63f214ca3 100644 --- a/core/actions/index.ts +++ b/core/actions/index.ts @@ -5,6 +5,7 @@ import { IncrementalTable } from "df/core/actions/incremental_table"; import { Notebook } from "df/core/actions/notebook"; import { Operation } from "df/core/actions/operation"; import { Table } from "df/core/actions/table"; +import { Test } from "df/core/actions/test"; import { View } from "df/core/actions/view"; import { IColumnsDescriptor } from "df/core/column_descriptors"; import { Resolvable } from "df/core/contextables"; @@ -19,7 +20,8 @@ export type Action = | Assertion | Declaration | Notebook - | DataPreparation; + | DataPreparation + | Test; export type ActionProto = | dataform.Table // core.proto's Table represents the Table, View or IncrementalTable action type. @@ -27,7 +29,8 @@ export type ActionProto = | dataform.Assertion | dataform.Declaration | dataform.Notebook - | dataform.DataPreparation; + | dataform.DataPreparation + | dataform.Test; // In v4, consider making methods on inheritors of this private, forcing users to use constructors // in order to populate actions. diff --git a/core/actions/test.ts b/core/actions/test.ts index b6d69d204..be19669e3 100644 --- a/core/actions/test.ts +++ b/core/actions/test.ts @@ -32,10 +32,13 @@ export interface ITestConfig extends INamedConfig { /** Path to the source file that the contents of the action is loaded from. */ filename?: string; + + /** Tags for this test action. */ + tags?: string[]; } /** @hidden */ -const ITestConfigProperties = strictKeysOf()(["type", "dataset", "name", "filename"]); +const ITestConfigProperties = strictKeysOf()(["type", "dataset", "name", "filename", "tags"]); /** * Dataform test actions can be used to write unit tests for your generated SQL @@ -77,8 +80,8 @@ export class Test extends ActionBuilder { /** @hidden We delay contextification until the final compile step, so hold these here for now. */ public contextableInputs = new Map>(); - private contextableQuery: Contextable; - private datasetToTest: Resolvable; + private contextableQuery: Contextable; + private testTarget: dataform.ITarget; /** * @hidden Stores the generated proto for the compiled graph. @@ -107,11 +110,34 @@ export class Test extends ActionBuilder { this.proto.name = config.name; } if (config.dataset) { - this.dataset(config.dataset); + // Determine target from the parent dataset name + this.testTarget = dataform.Target.create( + this.applySessionToTarget( + resolvableAsTarget( + toResolvable(config.dataset) + ), + this.session.projectConfig + ) + ); + const canonicalTestTarget = dataform.Target.create( + this.applySessionToTarget( + resolvableAsTarget( + toResolvable(config.dataset) + ), + this.session.canonicalProjectConfig + ) + ); + + // Set the target as the test name, with the tested action database and schema. + this.proto.target = overrideTargetWithNewName(this.testTarget, this.proto.name); + this.proto.canonicalTarget = overrideTargetWithNewName(canonicalTestTarget, this.proto.name); } if (config.filename) { this.proto.fileName = config.filename; } + if (config.tags) { + this.proto.tags = config.tags; + } return this; } @@ -119,7 +145,7 @@ export class Test extends ActionBuilder { * Sets the schema (BigQuery dataset) in which to create the output of this action. */ public dataset(ref: Resolvable) { - this.datasetToTest = ref; + this.config({ dataset: ref }); return this; } @@ -143,17 +169,19 @@ export class Test extends ActionBuilder { } /** @hidden */ - public getFileName() { + public getFileName(): string { return this.proto.fileName; } /** @hidden */ - public getTarget(): undefined { - // The test action type has no target because it is not processed during regular execution. - return undefined; + public getTarget(): dataform.Target { + return dataform.Target.create(this.proto.target); + } + + public getTestTarget(): dataform.Target { + return dataform.Target.create(this.testTarget); } - /** @hidden */ public setFilename(filename: string) { this.proto.fileName = filename; } @@ -161,23 +189,23 @@ export class Test extends ActionBuilder { /** @hidden */ public compile() { const testContext = new TestContext(this); - if (!this.datasetToTest) { + if (!this.testTarget) { this.session.compileError( new Error("Tests must operate upon a specified dataset."), this.proto.fileName ); } else { - const allResolved = this.session.indexedActions.find(resolvableAsTarget(this.datasetToTest)); + const allResolved = this.session.indexedActions.find(this.testTarget); if (allResolved.length > 1) { this.session.compileError( - new Error(ambiguousActionNameMsg(this.datasetToTest, allResolved)), + new Error(ambiguousActionNameMsg(this.testTarget, allResolved)), this.proto.fileName ); } const dataset = allResolved.length > 0 ? allResolved[0] : undefined; if (!(dataset && (dataset instanceof Table || dataset instanceof View))) { this.session.compileError( - new Error(`Dataset ${stringifyResolvable(this.datasetToTest)} could not be found.`), + new Error(`Dataset ${stringifyResolvable(this.testTarget)} could not be found.`), this.proto.fileName ); } else if (dataset instanceof IncrementalTable) { @@ -311,3 +339,11 @@ class RefReplacingContext implements ITableContext { return ""; } } + +function overrideTargetWithNewName(target: dataform.ITarget, testName: string): dataform.Target { + return dataform.Target.create({ + database: target.database, + schema: target.schema, + name: testName + }); +} diff --git a/core/actions/test_test.ts b/core/actions/test_test.ts index 9ddccf712..6d3e19e43 100644 --- a/core/actions/test_test.ts +++ b/core/actions/test_test.ts @@ -1,5 +1,379 @@ -import { suite } from "df/testing"; +// tslint:disable tsr-detect-non-literal-fs-filename +import { expect } from "chai"; +import * as fs from "fs-extra"; +import * as path from "path"; -suite("test", () => { - // This action currently has no unit tests! +import { asPlainObject, suite, test } from "df/testing"; +import { TmpDirFixture } from "df/testing/fixtures"; +import { + coreExecutionRequestFromPath, + runMainInVm, + VALID_WORKFLOW_SETTINGS_YAML +} from "df/testing/run_core"; + +suite("test", ({ afterEach }) => { + const tmpDirFixture = new TmpDirFixture(afterEach); + + test(`test with no inputs`, () => { + const projectDir = tmpDirFixture.createNewTmpDir(); + const workflowSettingsPath = path.join(projectDir, "workflow_settings.yaml"); + const definitionsDir = path.join(projectDir, "definitions"); + const actionsYamlPath = path.join(definitionsDir, "actions.yaml"); + const actionSqlPath = path.join(definitionsDir, "action.sql"); + const actionTestSqlxPath = path.join(definitionsDir, "action_test.sqlx"); + + fs.writeFileSync(workflowSettingsPath, VALID_WORKFLOW_SETTINGS_YAML); + fs.mkdirSync(definitionsDir); + fs.writeFileSync(actionsYamlPath, ` +actions: +- table: + filename: action.sql` + ); + fs.writeFileSync(actionSqlPath, "SELECT 1"); + fs.writeFileSync(actionTestSqlxPath, ` +config { + type: "test", + dataset: "action", + tags: ["tag1", "tag2"] +} +SELECT 1`); + + const result = runMainInVm(coreExecutionRequestFromPath(projectDir)); + + expect(result.compile.compiledGraph.graphErrors.compilationErrors).deep.equals([]); + expect(asPlainObject(result.compile.compiledGraph.tests)).deep.equals( + asPlainObject([ + { + // Original test properties + name: "action_test", + testQuery: "SELECT 1", + expectedOutputQuery: "\n\nSELECT 1", + fileName: "definitions/action_test.sqlx", + + // New properties + target: { + database: "defaultProject", + schema: "defaultDataset", + name: "action_test" + }, + canonicalTarget: { + database: "defaultProject", + schema: "defaultDataset", + name: "action_test" + }, + tags: ["tag1", "tag2"], + } + ]) + ); + expect(asPlainObject(result.compile.compiledGraph.tables)).deep.equals( + asPlainObject([ + { + "target": { + "database": "defaultProject", + "name": "action", + "schema": "defaultDataset" + }, + "canonicalTarget": { + "database": "defaultProject", + "name": "action", + "schema": "defaultDataset" + }, + "disabled": false, + "enumType": "TABLE", + "fileName": "definitions/action.sql", + "hermeticity": "NON_HERMETIC", + "query": "SELECT 1", + "type": "table" + } + ])); + }); + + test(`test with multiple_inputs input`, () => { + const projectDir = tmpDirFixture.createNewTmpDir(); + const workflowSettingsPath = path.join(projectDir, "workflow_settings.yaml"); + const definitionsDir = path.join(projectDir, "definitions"); + const actionsYamlPath = path.join(definitionsDir, "actions.yaml"); + const action1SqlxPath = path.join(definitionsDir, "action1.sqlx"); + const action1TestSqlxPath = path.join(definitionsDir, "action1_test.sqlx"); + const action2SqlxPath = path.join(definitionsDir, "action2.sqlx"); + const action2TestSqlxPath = path.join(definitionsDir, "action2_test.sqlx"); + + fs.writeFileSync(workflowSettingsPath, VALID_WORKFLOW_SETTINGS_YAML); + fs.mkdirSync(definitionsDir); + + // Add a declaration + fs.writeFileSync(actionsYamlPath, ` +actions: +- declaration: + name: a_declaration` + ); + + // Add an action with a test, reads from declaration + fs.writeFileSync(action1SqlxPath, ` +config { + type: "table", +} +SELECT a,b,c FROM \${ref("a_declaration")} + `); + fs.writeFileSync(action1TestSqlxPath, ` +config { + type: "test", + dataset: "action1" +} +input "a_declaration" { + SELECT 1 AS a, 2 AS b, 3 AS c, 4 AS d +} +SELECT 1 AS a, 2 AS b, 3 AS c`); + + + // Add an action with a test, reads from previous action + fs.writeFileSync(action2SqlxPath, ` +config { + type: "table", +} +SELECT a,b FROM \${ref("action1")} + `); + fs.writeFileSync(action2TestSqlxPath, ` +config { + type: "test", + dataset: "action2" +} +input "action1" { + SELECT 1 AS a, 2 AS b, 3 AS c +} +SELECT 1 AS a, 2 AS b`); + + const result = runMainInVm(coreExecutionRequestFromPath(projectDir)); + + expect(result.compile.compiledGraph.graphErrors.compilationErrors).deep.equals([]); + expect(asPlainObject(result.compile.compiledGraph.tests)).deep.equals( + asPlainObject([ + { + // Original test properties + name: "action1_test", + testQuery: "\n\nSELECT a,b,c FROM (\n SELECT 1 AS a, 2 AS b, 3 AS c, 4 AS d\n)\n ", + expectedOutputQuery: "\n\n\nSELECT 1 AS a, 2 AS b, 3 AS c", + fileName: "definitions/action1_test.sqlx", + + // New properties + target: { + database: "defaultProject", + schema: "defaultDataset", + name: "action1_test" + }, + canonicalTarget: { + database: "defaultProject", + schema: "defaultDataset", + name: "action1_test" + }, + }, + { + // Original test properties + name: "action2_test", + testQuery: "\n\nSELECT a,b FROM (\n SELECT 1 AS a, 2 AS b, 3 AS c\n)\n ", + expectedOutputQuery: "\n\n\nSELECT 1 AS a, 2 AS b", + fileName: "definitions/action2_test.sqlx", + + // New properties + target: { + database: "defaultProject", + schema: "defaultDataset", + name: "action2_test" + }, + canonicalTarget: { + database: "defaultProject", + schema: "defaultDataset", + name: "action2_test" + }, + } + ]) + ); + expect(asPlainObject(result.compile.compiledGraph.tables)).deep.equals( + asPlainObject([ + { + "target": { + "database": "defaultProject", + "name": "action1", + "schema": "defaultDataset" + }, + "canonicalTarget": { + "database": "defaultProject", + "name": "action1", + "schema": "defaultDataset" + }, + "dependencyTargets": [ + { + "database": "defaultProject", + "name": "a_declaration", + "schema": "defaultDataset" + } + ], + "disabled": false, + "enumType": "TABLE", + "fileName": "definitions/action1.sqlx", + "hermeticity": "NON_HERMETIC", + "query": "\n\nSELECT a,b,c FROM `defaultProject.defaultDataset.a_declaration`\n ", + "type": "table" + }, + { + "target": { + "database": "defaultProject", + "name": "action2", + "schema": "defaultDataset" + }, + "canonicalTarget": { + "database": "defaultProject", + "name": "action2", + "schema": "defaultDataset" + }, + "dependencyTargets": [ + { + "database": "defaultProject", + "name": "action1", + "schema": "defaultDataset" + } + ], + "disabled": false, + "enumType": "TABLE", + "fileName": "definitions/action2.sqlx", + "hermeticity": "NON_HERMETIC", + "query": "\n\nSELECT a,b FROM `defaultProject.defaultDataset.action1`\n ", + "type": "table" + } + ]) + ); + }); + + test(`test with two actions with same name and different schema`, () => { + const projectDir = tmpDirFixture.createNewTmpDir(); + const workflowSettingsPath = path.join(projectDir, "workflow_settings.yaml"); + const definitionsDir = path.join(projectDir, "definitions"); + const schema1actionSqlxPath = path.join(definitionsDir, "schema1_action.sqlx"); + const schema2actionSqlxPath = path.join(definitionsDir, "schema2_action.sqlx"); + const schema1actionTestSqlxPath = path.join(definitionsDir, "schema1_action_test.sqlx"); + const schema2actionTestSqlxPath = path.join(definitionsDir, "schema2_action_test.sqlx"); + + fs.writeFileSync(workflowSettingsPath, VALID_WORKFLOW_SETTINGS_YAML); + fs.mkdirSync(definitionsDir); + fs.writeFileSync(schema1actionSqlxPath, ` +config { + schema: "schema1", + name: "action", + type: "table", +} +SELECT 1 + `); + fs.writeFileSync(schema2actionSqlxPath, ` +config { + schema: "schema2", + name: "action", + type: "table", +} +SELECT 2 + `); + fs.writeFileSync(schema1actionTestSqlxPath, ` +config { + type: "test", + dataset: { + schema: "schema1", + name: "action", + }, +} +SELECT 1`); + fs.writeFileSync(schema2actionTestSqlxPath, ` +config { + type: "test", + dataset: { + schema: "schema2", + name: "action", + }, +} +SELECT 2`); + + const result = runMainInVm(coreExecutionRequestFromPath(projectDir)); + + expect(result.compile.compiledGraph.graphErrors.compilationErrors).deep.equals([]); + expect(asPlainObject(result.compile.compiledGraph.tests)).deep.equals( + asPlainObject([ + { + // Original test properties + name: "schema1_action_test", + testQuery: "\n\nSELECT 1\n ", + expectedOutputQuery: "\n\nSELECT 1", + fileName: "definitions/schema1_action_test.sqlx", + + // New properties + target: { + database: "defaultProject", + schema: "schema1", + name: "schema1_action_test" + }, + canonicalTarget: { + database: "defaultProject", + schema: "schema1", + name: "schema1_action_test" + }, + }, + { + // Original test properties + name: "schema2_action_test", + testQuery: "\n\nSELECT 2\n ", + expectedOutputQuery: "\n\nSELECT 2", + fileName: "definitions/schema2_action_test.sqlx", + + // New properties + target: { + database: "defaultProject", + schema: "schema2", + name: "schema2_action_test" + }, + canonicalTarget: { + database: "defaultProject", + schema: "schema2", + name: "schema2_action_test" + }, + }, + ]) + ); + expect(asPlainObject(result.compile.compiledGraph.tables)).deep.equals( + asPlainObject([ + { + "target": { + "database": "defaultProject", + "name": "action", + "schema": "schema1" + }, + "canonicalTarget": { + "database": "defaultProject", + "name": "action", + "schema": "schema1" + }, + "disabled": false, + "enumType": "TABLE", + "fileName": "definitions/schema1_action.sqlx", + "hermeticity": "NON_HERMETIC", + "query": "\n\nSELECT 1\n ", + "type": "table" + }, + { + "target": { + "database": "defaultProject", + "name": "action", + "schema": "schema2" + }, + "canonicalTarget": { + "database": "defaultProject", + "name": "action", + "schema": "schema2" + }, + "disabled": false, + "enumType": "TABLE", + "fileName": "definitions/schema2_action.sqlx", + "hermeticity": "NON_HERMETIC", + "query": "\n\nSELECT 2\n ", + "type": "table" + } + ])); + }); }); + diff --git a/core/session.ts b/core/session.ts index 8052de79f..3881aa26a 100644 --- a/core/session.ts +++ b/core/session.ts @@ -50,7 +50,9 @@ export class Session { public actions: Action[]; public indexedActions: ActionMap; - public tests: { [name: string]: Test }; + + // Tests need to be resolved after config is applied, which is why we keep them separate from other actions. + public tests: Action[]; // This map holds information about what assertions are dependent // upon a certain action in our actions list. We use this later to resolve dependencies. @@ -77,7 +79,7 @@ export class Session { dataform.ProjectConfig.create(originalProjectConfig || projectConfig || DEFAULT_CONFIG) ); this.actions = []; - this.tests = {}; + this.tests = []; this.graphErrors = { compilationErrors: [] }; } @@ -390,7 +392,7 @@ export class Session { newTest.session = this; newTest.setFilename(utils.getCallerFile(this.rootDir)); // Add it to global index. - this.tests[name] = newTest; + this.tests.push(newTest) return newTest; } @@ -426,10 +428,10 @@ export class Session { } public compile(): dataform.CompiledGraph { + this.actions.push(...this.tests); this.indexedActions = new ActionMap(this.actions); // defaultLocation is no longer a required parameter to support location auto-selection. - if ( !!this.projectConfig.vars && !Object.values(this.projectConfig.vars).every(value => typeof value === "string") @@ -454,7 +456,9 @@ export class Session { declarations: this.compileGraphChunk( this.actions.filter(action => action instanceof Declaration) ), - tests: this.compileGraphChunk(Object.values(this.tests)), + tests: this.compileGraphChunk( + this.actions.filter(action => action instanceof Test) + ), notebooks: this.compileGraphChunk(this.actions.filter(action => action instanceof Notebook)), dataPreparations: this.compileGraphChunk( this.actions.filter(action => action instanceof DataPreparation) @@ -470,7 +474,8 @@ export class Session { compiledGraph.assertions, compiledGraph.operations, compiledGraph.notebooks, - compiledGraph.dataPreparations + compiledGraph.dataPreparations, + compiledGraph.tests ) ); @@ -480,7 +485,8 @@ export class Session { compiledGraph.assertions, compiledGraph.operations, compiledGraph.notebooks, - compiledGraph.dataPreparations + compiledGraph.dataPreparations, + compiledGraph.tests ), [].concat(compiledGraph.declarations.map(declaration => declaration.target)) ); @@ -495,7 +501,8 @@ export class Session { compiledGraph.assertions, compiledGraph.operations, compiledGraph.notebooks, - compiledGraph.dataPreparations + compiledGraph.dataPreparations, + compiledGraph.tests ) ); verifyObjectMatchesProto( @@ -534,7 +541,7 @@ export class Session { return !!this.projectConfig.tablePrefix ? `${this.projectConfig.tablePrefix}_` : ""; } - private compileGraphChunk(actions: Array): T[] { + private compileGraphChunk(actions: Action[]): T[] { const compiledChunks: T[] = []; actions.forEach(action => { diff --git a/protos/core.proto b/protos/core.proto index ae9673205..fa5ff46d2 100644 --- a/protos/core.proto +++ b/protos/core.proto @@ -264,6 +264,13 @@ message Test { string test_query = 2; string expected_output_query = 3; + repeated string tags = 5; + + Target target = 6; + Target canonical_target = 7; + + repeated Target dependency_targets = 9; + // Generated. string file_name = 4; }