Skip to content

Find generated unittests - #10032

Open
lucia-vargas-a wants to merge 7 commits into
mainfrom
deg-11757_fing_generated_unittests_for_ci_validation
Open

lucia-vargas-a wants to merge 7 commits into
mainfrom
deg-11757_fing_generated_unittests_for_ci_validation

Conversation

@lucia-vargas-a

@lucia-vargas-a lucia-vargas-a commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Implements DENG-11757.

This PR:

  • Adds a check to the CI when sql-generators are changed and copies the unittests to the expected location when there is a change.
  • Ensures tests run only when the sql-generators change
  • Makes tests available for validate_metadata
  • Adds missing file in unittests which requires code owner approval.

Tests in this repo are not copied to private-bqetl.
The active_users_aggregates exceptions will be removed in a later PR, after this one is merged.

Reviewer, please follow this checklist

@lucia-vargas-a
lucia-vargas-a marked this pull request as ready for review October 14, 2026 17:47
@lucia-vargas-a
lucia-vargas-a requested a review from a team as a code owner October 14, 2026 17:47

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This PR adds a get_unittests_dir helper so the active_users_aggregates v3/v4 generators write unit tests next to their output directory. Before, tests for an absolute --output-dir landed under a relative tests/tmp/... path in the working directory. CI now packages those tests as a separate generated-unittests artifact for validate-metadata and deploy-changes-to-stage. The PR also moves the default metadata description into a constant. I traced the validate_metadata path change, and it gives the same result as the old parents[3]/parts[-4:] logic for both relative and absolute paths. My only comments are about how the CI step picks which generators to re-run.

Comment thread .github/workflows/build.yml Outdated
Comment thread .github/workflows/build.yml Outdated
@scholtzan

This comment has been minimized.

@mikewilli mikewilli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

r+wc

I think this PR should remove those tables from the skip list too.

Comment thread .github/workflows/build.yml
@scholtzan

This comment has been minimized.

@lucia-vargas-a

lucia-vargas-a commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor Author

r+wc

I think this PR should remove those tables from the skip list too.

PR to remove them - to merge after this one https://github.com/mozilla/bigquery-etl/pull/10036/changes

⏺ Add empty core_clients_last_seen test files required for focus_android's tests.
@lucia-vargas-a
lucia-vargas-a requested a review from a team as a code owner October 14, 2026 18:56
@lucia-vargas-a
lucia-vargas-a requested review from jdma-mozilla and kik-kik and removed request for jdma-mozilla October 14, 2026 18:56
@scholtzan

This comment has been minimized.

@scholtzan

Copy link
Copy Markdown
Contributor

Integration report

@lucia-vargas-a
lucia-vargas-a requested review from scholtzan and removed request for kik-kik October 15, 2026 14:05

This branch has not been deployed

No deployments
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.

3 participants