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

[CT-440] Adapter Test Framework Polish #4978

Closed
leahwicz opened this issue Mar 30, 2022 · 4 comments
Closed

[CT-440] Adapter Test Framework Polish #4978

leahwicz opened this issue Mar 30, 2022 · 4 comments
Labels
stale Issues that have gone stale

Comments

@leahwicz
Copy link
Contributor

leahwicz commented Mar 30, 2022

[edited by @jtcohen6]

Let's take a time-boxed second pass over the way we've factored reusable test utilities, fixtures, and functionality, just to make sure we're confident in the lay of the land. It might be perfect and need no changes. Just want to get a second set of eyes to confirm.

Review What Goes Where for:

  1. dbt.tests module, packaged as part of dbt-core, and usable by anyone who installs dbt-core
  2. Functional test cases (top-level tests/ directory in this repo), especially the fixtures + test cases that will be packaged up for adapters (dbt-tests-adapter)

Knowing that this interface will be semi-documented, for use by adapter plugin maintainers initially, and perhaps more package/project maintainers later on: Is there anything included in (1) that we should feel bad about including in stable, reliable, major-version-1 software (dbt-core)? We know we want to take another swing at the table-comparison logic: #4778

Also try to answer questions like:

  • Is the way we're presenting that separation in the docs (Initial docs for new adapter testing framework docs.getdbt.com#1263) sensible? Which reusable utilities do we feel comfortable documenting? (In that PR, we've just noted run_dbt)
  • Should we try turning the reusable test utilities (dbt.tests module) into an "extra" (pip install dbt-core[tests]), rather than included in the standard installation? I'm not actually sure if this is possible for package data, as opposed to optional dependencies
@github-actions github-actions bot changed the title Adapter Test Framework Polish [CT-440] Adapter Test Framework Polish Mar 30, 2022
@ChenyuLInx
Copy link
Contributor

After going through the tests case and documents a few times during the past week. I don't think there's anything we need to adjust for the current release. The new adapters tests covered the old test spec.
For the docs, I think they are very helpful and introduced things in a gradual way which is nice. One thing that I personally prefer is to have links to the example directly. But I think I haven't seen it in other places on our docs site. Are there concerns about doing that?
I don't think we should make the test utilities into "extra". I think they are mainly for declarition of additional dependencies right? Since we don't actually introduce any new dependency for the test utilities I think we are good.

@ChenyuLInx
Copy link
Contributor

ChenyuLInx commented Apr 13, 2022

Our new test framework reduced duplicate code in the adapters repo, also we can have a breakpoint for folks to use when running the tests(previous when writing tests I think most of the folks would have an example project on the side and reproduce the situation there instead of in the actual test code).
After talking to @dataders , I think one future work we can do is start thinking about how we can provide a better step-by-step experience to developers. One example he used was that the first thing when trying to develop an adapter is to setup the connection and make sure all the basic things there are implemented. I believe we currently don't have one specific tests for doing that. The functional test is a great abstraction for making sure the adapter has a solid overall experience. But for new adapter developers that don't have a solid dbt background, I think we need smaller tests/ guides for implementing the building blocks to get there and the order/dependency of those building blocks.

@github-actions
Copy link
Contributor

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please remove the stale label or comment on the issue, or it will be closed in 7 days.

@github-actions github-actions bot added the stale Issues that have gone stale label Oct 11, 2022
@github-actions
Copy link
Contributor

Although we are closing this issue as stale, it's not gone forever. Issues can be reopened if there is renewed community interest; add a comment to notify the maintainers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Issues that have gone stale
Projects
None yet
Development

No branches or pull requests

2 participants