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.

Sign up for GitHub

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

Jump to bottom

chore: Use helper methods in old unit tests #2119

Merged
merged 16 commits into from
Oct 16, 2023
Merged

chore: Use helper methods in old unit tests #2119

merged 16 commits into from
Oct 16, 2023

Conversation

sfc-gh-asawicki
Copy link
Collaborator

@sfc-gh-asawicki sfc-gh-asawicki commented Oct 13, 2023 •
edited
Loading

Use our helper methods assertOptsValidAndSQLEquals and assertOptsInvalid in older unit tests.

Changes

  • Replaced existing assertions with oneliner.
  • Fixed some of the tests and added some more validations. Many resources still need essential validations (issue incoming).
    • The main reason for tests failing was that the previous implementation was only creating opts and invoking structToSQL, whereas our helper functions are also calling validate(). So, some of the generated SQLs that were tested are not achievable through a normal execution and were not correct before. Wrong setups were also corrected.
  • Added some TODOs, which should be addressed later in another change (I will create an issue with the description).

@github-actions
Copy link

Integration tests success for 957adff0665e4904e3f578dc0486414ac7fd013f

@sfc-gh-asawicki sfc-gh-asawicki marked this pull request as ready for review October 13, 2023 10:50
@sfc-gh-asawicki sfc-gh-asawicki changed the title chore: Use helper methods in old tests chore: Use helper methods in old unit tests Oct 13, 2023
@github-actions
Copy link

Integration tests failure for f12ecdb4ca7a626b877a86e4d26b7877f3941077

1 similar comment
@github-actions
Copy link

Integration tests failure for f12ecdb4ca7a626b877a86e4d26b7877f3941077

sfc-gh-jcieslak
sfc-gh-jcieslak reviewed Oct 13, 2023
View reviewed changes
pkg/sdk/file_format_test.go Show resolved Hide resolved
pkg/sdk/file_format_test.go Show resolved Hide resolved
pkg/sdk/file_format_test.go Show resolved Hide resolved
@github-actions
Copy link

Integration tests failure for f12ecdb4ca7a626b877a86e4d26b7877f3941077

@github-actions
Copy link

Integration tests failure for 7a804551e1deb45e43ea71be62c48bbb388033c7

@github-actions
Copy link

Integration tests success for 439cbade56a80e39efd6ad467701f4c9828d1e20

@sfc-gh-asawicki sfc-gh-asawicki merged commit 0c44571 into main Oct 16, 2023
8 checks passed
@sfc-gh-asawicki sfc-gh-asawicki deleted the use-helper-methods-in-old-tests branch October 16, 2023 10:13
@snowflake-release-please snowflake-release-please bot mentioned this pull request Oct 16, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sfc-gh-jcieslak sfc-gh-jcieslak sfc-gh-jcieslak approved these changes

@sfc-gh-swinkler sfc-gh-swinkler Awaiting requested review from sfc-gh-swinkler

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sfc-gh-asawicki @sfc-gh-jcieslak