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

Beginning to reorg the functional tests #492

Merged
merged 1 commit into from
Nov 10, 2023
Merged

Beginning to reorg the functional tests #492

merged 1 commit into from
Nov 10, 2023

Conversation

benc-db
Copy link
Collaborator

@benc-db benc-db commented Oct 27, 2023

Description

To make it easier to determine which functional tests we're missing, I'm adopting the organizational strategy of the adapter folder in core. From migrating these tests, I found that I had added a duplicate test in the last PR because of the difficulty of tracking which tests we have vs which we're missing.

Couple of other notes

  • Added alias tests we were missing
  • test_basic.py has been exploded into a lot of really simple files, primarily to keep alignment with core, but also because sometimes we have a situation like with doc generation where we need a large override, and I don't like python files that feel like a grab bag of unrelated things.
  • I've started moving Databricks to the front of the test class names. I prefer to have the stuff we want to ignore (TestDatabricks) in one place, rather than sandwiching them around the part of the test name that has meaning.

Checklist

  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt-databricks next" section.

@benc-db
Copy link
Collaborator Author

benc-db commented Oct 27, 2023

For discussion: Do we need Databricks in the name at all? In cases where dbt doesn't have a base for us to inherit its necessary, but I think when they do that, it's more an accident due to time crunch than an intentional choice.

Copy link

@susodapop susodapop left a comment

Choose a reason for hiding this comment

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

LGTM. I checked these out locally and explored in the VSCode test explorer and they are much easier to walk through 👍

Should we include "Databricks" in the test cases?
I think no. If we need to override a test case in dbt-core or dbt-spark we can add "Override" to the test case to make it more clear what's happening and why.

@benc-db benc-db merged commit 6e42d38 into main Nov 10, 2023
21 checks passed
benc-db added a commit that referenced this pull request Nov 13, 2023
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