Skip to content
Open
7 changes: 5 additions & 2 deletions core/actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -19,15 +20,17 @@ export type Action =
| Assertion
| Declaration
| Notebook
| DataPreparation;
| DataPreparation
| Test;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The addition of tests to the list of targets in the compiled graph and a dependency, changes in session.test backing structure is technically a breaking change.

At the very least we should bump a minor version, once we merger theses changes.

Copy link
Collaborator Author

@fernst fernst Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've since removed the dependency changes (meaning tests are in the DAG, but not connected to any other action).

Should we still do the minor version bump? Or can we keep it under patch versions until we add the new tests into the middle of the DAG?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment there are still breaking changes in Dataform Core API, specifically in Session object. Bottom line is these changes shouldn't be released under a patch, which means that we should either:

  1. Exercise control and block all @dataform/core releases until all breaking changes are submitted from the feature branch
  2. Prepare all breaking changes in a separate branch and them merge them in a single commit with a minor version bump.

At the moment we don't have any automation to enforce (1) repo-wide. I think executing (2) is easier.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current changes LGTM from my side, will wait for resolution in this thread to approve the PR


export type ActionProto =
| dataform.Table // core.proto's Table represents the Table, View or IncrementalTable action type.
| dataform.Operation
| 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.
Expand Down
64 changes: 50 additions & 14 deletions core/actions/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ITestConfig>()(["type", "dataset", "name", "filename"]);
const ITestConfigProperties = strictKeysOf<ITestConfig>()(["type", "dataset", "name", "filename", "tags"]);

/**
* Dataform test actions can be used to write unit tests for your generated SQL
Expand Down Expand Up @@ -77,8 +80,8 @@ export class Test extends ActionBuilder<dataform.Test> {

/** @hidden We delay contextification until the final compile step, so hold these here for now. */
public contextableInputs = new Map<string, Contextable<IActionContext, string>>();
private contextableQuery: Contextable<IActionContext, string>;
private datasetToTest: Resolvable;
private contextableQuery: Contextable<IActionContext, string>;
private testTarget: dataform.ITarget;

/**
* @hidden Stores the generated proto for the compiled graph.
Expand Down Expand Up @@ -107,19 +110,42 @@ export class Test extends ActionBuilder<dataform.Test> {
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;
}

/**
* 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;
}

Expand All @@ -143,41 +169,43 @@ export class Test extends ActionBuilder<dataform.Test> {
}

/** @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;
}

/** @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) {
Expand Down Expand Up @@ -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
});
}
Loading
Loading