Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore ability to unit test views #1824

Merged
merged 4 commits into from
Sep 5, 2024
Merged

Conversation

bmagyarkuti
Copy link
Contributor

@bmagyarkuti bmagyarkuti commented Sep 2, 2024

Prior to release 3.0.1, it was possible to unit test views. For example:

test("test a view")
  .dataset("example_view")
  .input(
    "sample_data",
    "select 'hi' as col1, 1 as col2, 3.5 as col3, true as col4, date '2020-07-23' as col5"
  )
  .expect(
    "select 'hi' as col1, 1 as col2, 3.5 as col3, true as col4, date '2020-07-23' as col5"
  );

Such a unit test always fails after 3.0.1 (including 3.0.2). It produces the following error message:
Dataset sample_data could not be found. This happens regardless of whether the view has been defined.

This PR re-introduces the ability to unit test views.

I'd like to nominate @Ekrekr as reviewer.

@bmagyarkuti bmagyarkuti changed the title Restore ability to run unit tests Restore ability to unit test views Sep 2, 2024
@lveraszto
Copy link

+1

Copy link
Contributor

@Ekrekr Ekrekr left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

core/actions/test.ts Outdated Show resolved Hide resolved
tests/integration/bigquery_project/definitions/test.js Outdated Show resolved Hide resolved
barna-emarsys and others added 2 commits September 3, 2024 11:22
Co-authored-by: Elias Kassell <eliaskassell@gmail.com>
@bmagyarkuti
Copy link
Contributor Author

@Ekrekr I've incorporated both your suggested changes. Is there anything else I need to do before this can be merged?

@Ekrekr
Copy link
Contributor

Ekrekr commented Sep 5, 2024

Thanks - I've run the tests and they're all good, I just need you to sign the CLA before I can merge.

@barna-emarsys
Copy link
Contributor

barna-emarsys commented Sep 5, 2024

@Ekrekr, when I accepted your suggested changes, you became a co-author on that commit, and your @gmail address seems to be missing a signed CLA, so it seems you now need to sign the CLA. Sorry about this, maybe it was a bad idea to use Github's built-in "accept changes" functionality, let me know if you have any ideas for a workaround.

@Ekrekr
Copy link
Contributor

Ekrekr commented Sep 5, 2024

Huh weird, I see that too, but it also shows as me having already signed it when I follow the link. Will just merge through it!

@Ekrekr Ekrekr merged commit 1f48a0e into dataform-co:main Sep 5, 2024
0 of 2 checks passed
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.

4 participants