Populate new unit test properties and add flag to include unit tests in compiled graph#2063
Populate new unit test properties and add flag to include unit tests in compiled graph#2063
Conversation
fernst
force-pushed
the
test-proto-changes
branch
2 times, most recently
from
512080f to
6f6b3ee
Compare
kolina
left a comment
kolina
left a comment
There was a problem hiding this comment.
Can you please against the latest main? So integration tests will run with non-expired credentials
fernst
force-pushed
the
test-proto-changes
branch
from
149c2fe to
72903be
Compare
fernst
force-pushed
the
test-proto-changes
branch
from
0dc9788 to
b03d245
Compare
| ); | ||
| } | ||
|
|
||
| private overrideTargetWithNewName(target: dataform.ITarget, testName: string): dataform.Target { |
There was a problem hiding this comment.
Nit: Can be a local function, as it doesn't depend on this.
| const fullyQualifiedDependencies: { [name: string]: dataform.ITarget } = {}; | ||
| if (action instanceof dataform.Declaration || !action.dependencyTargets) { | ||
| // Declarations cannot have dependencies. | ||
| // Declarations cannot have dependencies. |
There was a problem hiding this comment.
Nit: dangling whitespace?
| | Notebook | ||
| | DataPreparation; | ||
| | DataPreparation | ||
| | Test; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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:
- Exercise control and block all @dataform/core releases until all breaking changes are submitted from the feature branch
- 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.
There was a problem hiding this comment.
Current changes LGTM from my side, will wait for resolution in this thread to approve the PR
There was a problem hiding this comment.
I've added a new configuration property to only add the unit tests to the compiled DAG when this setting is set. This ensures backwards compatibility and ensures the graph remains consistent for users who want to keep this behavior.
kolina
left a comment
kolina
left a comment
There was a problem hiding this comment.
Waiting on resolution of how to update Dataform core version with these changes
fernst
force-pushed
the
test-proto-changes
branch
from
ce2591f to
c3fb0a8
Compare
… the compiled DAG as dependencies to the action being tested
fernst
changed the title
3 participants
Populated new properties in unit test proto during compilation.
Also added a new workflow config setting to include unit tests in compiled DAG.