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

Reorganize tests to match their code location #10191

Merged
merged 11 commits into from
May 21, 2024
Merged

Reorganize tests to match their code location #10191

merged 11 commits into from
May 21, 2024

Conversation

ChenyuLInx
Copy link
Contributor

resolves ##9961

Problem

Our unit test does not match the code location. This makes finding example unit tests harder.

Solution

We are moving all unit tests to match the code location they are at.

@ChenyuLInx ChenyuLInx requested a review from a team as a code owner May 21, 2024 05:07
@cla-bot cla-bot bot added the cla:yes label May 21, 2024
from tests.unit.utils import ContractTestCase


class TestUnparsedMacro(ContractTestCase):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved from test_contracts_graph_unparsed

Copy link

codecov bot commented May 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.57%. Comparing base (09243d1) to head (9bceb37).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10191      +/-   ##
==========================================
- Coverage   88.63%   88.57%   -0.07%     
==========================================
  Files         180      180              
  Lines       22434    22434              
==========================================
- Hits        19885    19870      -15     
- Misses       2549     2564      +15     
Flag Coverage Δ
integration 85.93% <ø> (-0.16%) ⬇️
unit 63.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to graph/test_selectors.py

Copy link
Contributor Author

@ChenyuLInx ChenyuLInx May 21, 2024

Choose a reason for hiding this comment

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

Not sure why this does not show up in the file view, but this file does exists

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to graph/test_selector_spec.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to graph/test_nodes.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to contracts/graph/test_nodes.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to config/test_selectors.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moving to dbt-common pr

@ChenyuLInx ChenyuLInx requested a review from QMalcolm May 21, 2024 16:52
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to contracts/graph/test_unparsed.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This we converted from unit test to pytest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changing from unit test to pytest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

consolidated test_manifest_selectors.py to this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consolidated test_inject_ctes.py to this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consolidated test_infer_primary_key.py to this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved from test_graph_selector_spec.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to contracts/graph/test_unparsed.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to graph/test_selector_spec.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to graph/test_nodes.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to contracts/graph/test_nodes.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to config/test_selectors.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to dbt-common

Copy link
Contributor

@QMalcolm QMalcolm left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you for doing this work 🙏 I do have one concern about a test that I think got deleted in the shuffle as well as a couple items we can decide to follow up on later.

@@ -55,8 +55,7 @@
)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think an argument could be made that these tests should be moved into other files. There are config tests that should probably exist in /tests/unit/contracts/graph/test_model_config.py and the rest can probably be moved to /tests_unit_contracts/graph/test_nodes.py. However perhaps that should be a follow up task given this file is 2347 lines long

tests/unit/contracts/graph/test_unparsed.py Outdated Show resolved Hide resolved
tests/unit/parser/test_unit_tests.py Outdated Show resolved Hide resolved
@ChenyuLInx ChenyuLInx requested a review from QMalcolm May 21, 2024 20:44
Copy link
Contributor

@QMalcolm QMalcolm left a comment

Choose a reason for hiding this comment

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

LGTM!

@ChenyuLInx ChenyuLInx merged commit ad61200 into main May 21, 2024
60 checks passed
@ChenyuLInx ChenyuLInx deleted the cl/re-org-test branch May 21, 2024 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants