Skip to content

fix: error messages when compiling tests for incremental tables#2118

Open
ashish10alex wants to merge 1 commit intodataform-co:mainfrom
ashish10alex:fix_incremental_check
Open

fix: error messages when compiling tests for incremental tables#2118
ashish10alex wants to merge 1 commit intodataform-co:mainfrom
ashish10alex:fix_incremental_check

Conversation

@ashish10alex
Copy link
Contributor

This PR:

  1. Solves: bug: incorrect error message when adding tests for incremental table type #2117

  2. Avoid "Test query is empty." error for the same file by avoiding this check and returning this.proto early.

    if (!this.proto.testQuery.trim()) {

  3. Added unit test for expected behavior

Tests

  • bazel test //core:actions/test_test passes
  • bazel test //core/... passes
  • Manually tested by building the core package and using it in a local repository. See screenshot below
CleanShot 2026-03-19 at 15 44 47@2x

@ashish10alex ashish10alex requested a review from a team as a code owner March 19, 2026 15:46
@ashish10alex ashish10alex requested review from kolina and removed request for a team March 19, 2026 15:46
@kolina
Copy link
Contributor

kolina commented Mar 21, 2026

/gcbrun

new Error(ambiguousActionNameMsg(this.datasetToTest, allResolved)),
this.proto.fileName
);
return this.proto;
Copy link
Contributor

Choose a reason for hiding this comment

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

It may still be useful to collect all compilation errors. But I'm fine to have early return as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I initially had it collect everything. However, that resulted in confusing compilation message such as ""Test query is empty." being added for the user due to code trying to parse query object which will not be populated. Which is why opted for the early return.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants