-
Notifications
You must be signed in to change notification settings - Fork 121
Add setup for DBR testing #3557
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
base: main
Are you sure you want to change the base?
Conversation
| // If the test is being run on DBR, auth is already configured | ||
| // by the dbr_runner notebook by reading a token from the notebook context and | ||
| // setting DATABRICKS_TOKEN and DATABRICKS_HOST environment variables. | ||
| _, _, tmpDir = workspaceTmpDir(t.Context(), t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was necesary to ensure the test directory gets cleaned up. Otherwise the context seems to be cancelled before t.Cleanup is executed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The doc for testing.T.Context says as much: https://pkg.go.dev/testing#T.Context
Unless there is another context to use, it seems better to use context.Background() in the cleanup function directly, or other call sites will be prone to the same error.
14 failing tests:
|
pietern
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can live in a dbr subpackage under acceptance for more isolation.
| // If the test is being run on DBR, auth is already configured | ||
| // by the dbr_runner notebook by reading a token from the notebook context and | ||
| // setting DATABRICKS_TOKEN and DATABRICKS_HOST environment variables. | ||
| _, _, tmpDir = workspaceTmpDir(t.Context(), t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The doc for testing.T.Context says as much: https://pkg.go.dev/testing#T.Context
Unless there is another context to use, it seems better to use context.Background() in the cleanup function directly, or other call sites will be prone to the same error.
NEXT_CHANGELOG.md
Outdated
|
|
||
| ### Bundles | ||
| * Remove resources.apps.config section ([#3680](https://github.com/databricks/cli/pull/3680)) | ||
| * Prompt for serverless compute in `dbt-sql` template (defaults to `yes`) ([#3668](https://github.com/databricks/cli/pull/3668)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check, I think you're accidentally removing several changes from main (both in the changelog and actual changes in for example libs/template). Resolving conflicts might fix this.
489f91e to
c5f44a3
Compare
|
This PR has not received an update in a while. If you want to keep this PR open, please leave a comment below or push a new commit and auto-close will be canceled. |
Changes
This PR adds a notebook we can use to easily run and iterate on DBR tests.
Why
Simplifies the test setup and makes it easy to debug issues with the DBR testing framework itself. You just have to run
make dbr_scratchto get started.