Move custom variables test to the new testing structure (dataform-co#…

…1601)

* Update tests for custom variables to new structure, enforce it for workflow_settings.yaml too

* Tidy

* Fix tslint

common/protos/index.ts

@@ -15,6 +15,8 @@ export interface IProtoClass<IProto, Proto> {
 }
 
 // ProtobufJS's native verify method does not check that only defined fields are present.
+// Note: ProtobufJS does not do conversion of typed fields, so if a type number is present but a
+// string expected, this won't throw an error.
 // TODO(ekrekr): swap to Typescript protobuf library rather than having to do this; however using TS
 // libraries currently available would incur a significant performance hit.
 export function verifyObjectMatchesProto<Proto>(

core/main_test.ts

@@ -152,6 +152,119 @@ suite("@dataform/core", ({ afterEach }) => {
       );
     });
 
+    suite("variables", () => {
+      test(`variables in workflow_settings.yaml must be strings`, () => {
+        const projectDir = tmpDirFixture.createNewTmpDir();
+        // tslint:disable-next-line: tsr-detect-non-literal-fs-filename
+        fs.writeFileSync(
+          path.join(projectDir, "workflow_settings.yaml"),
+          `
+vars:
+  intValue: 1
+  strValue: "str"`
+        );
+        const coreExecutionRequest = dataform.CoreExecutionRequest.create({
+          compile: { compileConfig: { projectDir } }
+        });
+
+        expect(() => runMainInVm(coreExecutionRequest)).to.throw(
+          "Custom variables defined in workflow settings can only be strings."
+        );
+      });
+
+      test(`variables in dataform.json must be strings`, () => {
+        const projectDir = tmpDirFixture.createNewTmpDir();
+        // tslint:disable-next-line: tsr-detect-non-literal-fs-filename
+        fs.writeFileSync(
+          path.join(projectDir, "dataform.json"),
+          `{"vars": { "intVar": 1, "strVar": "str" } }`
+        );
+        const coreExecutionRequest = dataform.CoreExecutionRequest.create({
+          compile: { compileConfig: { projectDir } }
+        });
+
+        expect(() => runMainInVm(coreExecutionRequest)).to.throw(
+          "Custom variables defined in workflow settings can only be strings."
+        );
+      });
+
+      test(`variables can be referenced in SQLX`, () => {
+        const projectDir = tmpDirFixture.createNewTmpDir();
+        // tslint:disable-next-line: tsr-detect-non-literal-fs-filename
+        fs.writeFileSync(
+          path.join(projectDir, "workflow_settings.yaml"),
+          `
+defaultLocation: "us"
+vars:
+  var1: value1
+  var2: value2
+  var3: value3`
+        );
+        // tslint:disable-next-line: tsr-detect-non-literal-fs-filename
+        fs.mkdirSync(path.join(projectDir, "definitions"));
+        // tslint:disable-next-line: tsr-detect-non-literal-fs-filename
+        fs.writeFileSync(
+          path.join(projectDir, "definitions/file.sqlx"),
+          // TODO(https://github.com/dataform-co/dataform/issues/1295): add a test and fix
+          // functionality for assertions overriding database.
+          `
+config {
+  type: "table",
+  database: dataform.projectConfig.vars.var1,
+  description: dataform.projectConfig.vars.var2
+}
+select 1 AS \${dataform.projectConfig.vars.var3}`
+        );
+        const coreExecutionRequest = dataform.CoreExecutionRequest.create({
+          compile: { compileConfig: { projectDir, filePaths: ["definitions/file.sqlx"] } }
+        });
+
+        const result = runMainInVm(coreExecutionRequest);
+
+        expect(asPlainObject(result.compile.compiledGraph)).deep.equals(
+          asPlainObject({
+            dataformCoreVersion: "3.0.0",
+            graphErrors: {},
+            projectConfig: {
+              defaultLocation: "us",
+              vars: {
+                var1: "value1",
+                var2: "value2",
+                var3: "value3"
+              },
+              warehouse: "bigquery"
+            },
+            tables: [
+              {
+                actionDescriptor: {
+                  description: "value2"
+                },
+                canonicalTarget: {
+                  database: "value1",
+                  name: "file"
+                },
+                disabled: false,
+                enumType: "TABLE",
+                fileName: "definitions/file.sqlx",
+                query: "\n\nselect 1 AS value3",
+                target: {
+                  database: "value1",
+                  name: "file"
+                },
+                type: "table"
+              }
+            ],
+            targets: [
+              {
+                database: "value1",
+                name: "file"
+              }
+            ]
+          })
+        );
+      });
+    });
+
     // TODO(ekrekr): add a test for nested fields, once they exist.
   });
 
@@ -170,8 +283,7 @@ suite("@dataform/core", ({ afterEach }) => {
         path.join(projectDir, "definitions/actions.yaml"),
         `
 actions:
-- fileName: definitions/notebook.ipynb
-`
+  - fileName: definitions/notebook.ipynb`
       );
       // tslint:disable-next-line: tsr-detect-non-literal-fs-filename
       fs.writeFileSync(

core/session.ts

@@ -359,7 +359,7 @@ export class Session {
       !!this.config.vars &&
       !Object.values(this.config.vars).every(value => typeof value === "string")
     ) {
-      throw new Error("Custom variables defined in dataform.json can only be strings.");
+      throw new Error("Custom variables defined in workflow settings can only be strings.");
     }
 
     // TODO(ekrekr): replace verify here with something that actually works.

core/workflow_settings.ts

@@ -21,34 +21,39 @@ export function readWorkflowSettings(): dataform.ProjectConfig {
     if (!workflowSettingsAsJson) {
       throw Error("workflow_settings.yaml is invalid");
     }
-    verifyWorkflowSettingsAsJson(workflowSettingsAsJson);
-    return dataform.ProjectConfig.fromObject(workflowSettingsAsJson);
+    return verifyWorkflowSettingsAsJson(workflowSettingsAsJson);
   }
 
   if (dataformJson) {
-    verifyWorkflowSettingsAsJson(dataformJson);
-    return dataform.ProjectConfig.fromObject(dataformJson);
+    return verifyWorkflowSettingsAsJson(dataformJson);
   }
 
   throw Error("Failed to resolve workflow_settings.yaml");
 }
 
-function verifyWorkflowSettingsAsJson(workflowSettingsAsJson: object): { [key: string]: any } {
+function verifyWorkflowSettingsAsJson(workflowSettingsAsJson: object): dataform.ProjectConfig {
+  let projectConfig = dataform.ProjectConfig.create();
   try {
-    verifyObjectMatchesProto(dataform.ProjectConfig, workflowSettingsAsJson);
+    projectConfig = dataform.ProjectConfig.create(
+      verifyObjectMatchesProto(
+        dataform.ProjectConfig,
+        workflowSettingsAsJson as {
+          [key: string]: any;
+        }
+      )
+    );
   } catch (e) {
     if (e instanceof ReferenceError) {
       throw ReferenceError(`Workflow settings error: ${e.message}`);
     }
     throw e;
   }
-  const verifiedWorkflowSettings = workflowSettingsAsJson as { [key: string]: any };
   // tslint:disable-next-line: no-string-literal
-  if (!verifiedWorkflowSettings["warehouse"]) {
+  if (!projectConfig.warehouse) {
     // tslint:disable-next-line: no-string-literal
-    verifiedWorkflowSettings["warehouse"] = "bigquery";
+    projectConfig.warehouse = "bigquery";
   }
-  return verifiedWorkflowSettings;
+  return projectConfig;
 }
 
 function maybeRequire(file: string): any {

tests/core/core.spec.ts

@@ -1079,35 +1079,6 @@ suite("@dataform/core", () => {
         "A defaultLocation is required for BigQuery. This can be configured in dataform.json."
       ]);
     });
-
-    test("variables defined in dataform.json must be strings", () => {
-      const sessionFail = new Session(path.dirname(__filename), {
-        warehouse: "bigquery",
-        defaultSchema: "schema",
-        defaultLocation: "location",
-        vars: {
-          int_var: 1,
-          str_var: "str"
-        }
-      } as any);
-
-      expect(() => {
-        sessionFail.compile();
-      }).to.throw("Custom variables defined in dataform.json can only be strings.");
-
-      const sessionSuccess = new Session(path.dirname(__filename), {
-        warehouse: "bigquery",
-        defaultSchema: "schema",
-        defaultLocation: "location",
-        vars: {
-          str_var1: "str1",
-          str_var2: "str2"
-        }
-      } as any);
-
-      const graph = sessionSuccess.compile();
-      expect(graph.graphErrors.compilationErrors).to.eql([]);
-    });
   });
 
   suite("compilers", () => {