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

Point testing dependencies to main #12

Merged
merged 7 commits into from
Feb 15, 2024
Merged

Conversation

mikealfare
Copy link
Contributor

@mikealfare mikealfare commented Feb 12, 2024

All testing dependencies should point to main to ensure that the latest changes are reflected.

  • Move all optional dependencies into hatch environments
    • They aren't production dependencies
    • We can't install from GitHub in optional dependencies and also publish to PyPI
  • Make the default environment the dev version of all production dependencies
  • Inherit from the default environment for non-detached hatch environments (typechecking and testing)
  • Pin lower bound for dependencies in alignment with our practices

Checklist

  • I have read the contributing guide and understand what's expected of me
  • 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
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX

@mikealfare mikealfare self-assigned this Feb 12, 2024
@mikealfare mikealfare marked this pull request as ready for review February 12, 2024 22:52
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test cases added here are being migrated back to dbt-postgres (for now). It was determined that the issue was likely being caused by a combination of not looking at main for internal dependencies and not instantiating the invocation context variables before checking them. set_invocation_context({}) is run on line 338 now, right before getting the catalog, and tests are now passing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dependencies were moved down into the hatch environments, in alignment with dbt-adapters. This was done for a combination of reasons:

  • we cannot publish a package to PyPI with dependencies installed from GitHub, so development versions of internal packages would need to be identified as hatch environment dependencies
  • there is not a lot of value in installing dev and test tooling without the development versions of internal packages
  • having just the tooling as installable extras in the production distribution is confusing

features = ["dev", "test"]
dependencies = [
# TODO: remove `dbt-core` dependencies from unit tests
"dbt-core @ git+https://github.com/dbt-labs/dbt-core.git#subdirectory=core",
Copy link
Contributor

Choose a reason for hiding this comment

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

kinda surprised we don't need dbt-tests-adapter for unit tests

@mikealfare mikealfare merged commit 5fa52ca into main Feb 15, 2024
9 checks passed
@mikealfare mikealfare deleted the tests/experimental-parser branch February 15, 2024 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants