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

Refactoring to make testing easier & adding tests #96

Merged
merged 13 commits into from
Sep 21, 2023

Conversation

syou6162
Copy link
Contributor

@syou6162 syou6162 commented Sep 17, 2023

Why

What

@z3z1ma

  • To adding tests easier, I split functions to another file(src/dbt_osmosis/core/column_level_knowledge_propagator.py)
    • Defined types for ColumnLevelKnowledge and Knowledge to understand code easier
  • Adding some tests
    • In this pull request, we are only adding functions for which tests are relatively easy to add (to avoid making the differences in this pull request too large and difficult to review)
    • We intend to add more detailed tests and tests for other functions in subsequent pull requests
  • Run pytest on GitHub Actions

@@ -0,0 +1,11347 @@
{
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 made this file with dbt build --target test in demo_duckdb directory.

@syou6162 syou6162 changed the title WIP: Add tests Refactoring to make testing easier & adding tests Sep 17, 2023
@syou6162 syou6162 marked this pull request as ready for review September 17, 2023 07:41
Copy link
Owner

@z3z1ma z3z1ma left a comment

Choose a reason for hiding this comment

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

Very nice improvements, clean code. 💯

LGTM

@z3z1ma z3z1ma merged commit 0c2c8d6 into z3z1ma:main Sep 21, 2023
1 check passed
@syou6162 syou6162 deleted the add_tests branch September 21, 2023 03:55
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