## Changes
This PR:
1. Incrementally improves the error messages shown to the user when the
volume they are referring to in `workspace.artifact_path` does not
exist.
2. Performs this validation in both `bundle validate` and `bundle
deploy` compared to before on just deployments.
3. It runs "fast" validations on `bundle deploy`, which earlier were
only run on `bundle validate`.
## Tests
Unit tests and manually. Also, existing integration tests provide
coverage (`TestUploadArtifactToVolumeNotYetDeployed`,
`TestUploadArtifactFileToVolumeThatDoesNotExist`)
Examples:
```
.venv➜ bundle-playground git:(master) ✗ cli bundle validate
Error: cannot access volume capital.whatever.my_volume: User does not have READ VOLUME on Volume 'capital.whatever.my_volume'.
at workspace.artifact_path
in databricks.yml:7:18
```
and
```
.venv➜ bundle-playground git:(master) ✗ cli bundle validate
Error: volume capital.whatever.foobar does not exist
at workspace.artifact_path
resources.volumes.foo
in databricks.yml:7:18
databricks.yml:12:7
You are using a volume in your artifact_path that is managed by
this bundle but which has not been deployed yet. Please first deploy
the volume using 'bundle deploy' and then switch over to using it in
the artifact_path.
```
## Changes
https://github.com/databricks/mlops-stacks/pull/187 broke mlops-stacks
deployments for non-UC projects. Snoozing the test until upstream is
fixed.
## Tests
The test is skipped on my local machine. CI run will verify it's also
skipped on Github Actions runners.
## Changes
- Enable new linter: testifylint.
- Apply fixes with --fix.
- Fix remaining issues (mostly with aider).
There were 2 cases we --fix did the wrong thing - this seems to a be a
bug in linter: https://github.com/Antonboom/testifylint/issues/210
Nonetheless, I kept that check enabled, it seems useful, just need to be
fixed manually after autofix.
## Tests
Existing tests
## Changes
- Detach "golden files" assertions from testcli runner. Now any output
can be compared, no matter how it is obtained.
- Move those assertion to libs/testdiff package.
This allows using "golden files" in non-integration tests.
## Tests
Existing tests
## Changes
Add new type of test helpers that run the command and compare full
output (golden files approach).
In case of JSON, there is also an option to ignore certain paths.
Add test for different versions of Python to go through bundle init
default-python / validate / deploy / summary.
## Tests
New integration tests.
## Changes
There was only one helper for AWS and not the other clouds. Found this
when looking through double calls to `acc.WorkspaceTest()` (see
`TestPythonWheelTaskDeployAndRunOnInteractiveCluster`).
## Tests
n/a
## Changes
A small subset of tests trigger cluster creation to run jobs. These
tests comprise a substantial amount of the total integration test
runtime. We can skip them on PRs and only run them on the main branch.
## Tests
Confirmed the short runtime is ~20 mins.
## Changes
The `acc` package is exclusively used by integration tests, so it
belongs under `integration/internal`.
It's not the best name we can rename later.
## Tests
n/a
## Changes
These test helpers were updated to handle the error internally and not
return it. Since they have testing.T object, they can do so directly. On
the caller side, this functions were always followed by
require.NoError(t, err), that was cleaned up.
This approach helps reduce the setup/teardown boilerplate in the test
cases.
## Tests
Existing tests.
## Changes
The `Setenv` helper function configures an environment variable and
resets it to its original value when exiting the test scope. It is
incompatible with running tests in parallel because it modifies
process-wide state. The `libs/env` package defines functions to interact
with the environment but records `Setenv` calls on a `context.Context`.
This enables us to override/specialize the environment scoped to a
context.
Pre-requisites for removing the `t.Setenv` calls:
* Make `cmdio.NewIO` accept a context and use it with `libs/env`
* Make all `internal/testcli` functions use a context
The rest of this change:
* Modifies integration tests to initialize a context to use if there
wasn't already one
* Updates `t.Setenv` calls to use `env.Set`
## Tests
n/a
## Changes
These calls are no longer necessary now that integration tests use a
main function that performs this check. This change updates integration
tests that call this function. Of those, the call sites that initialize
a workspace client are updated to use `acc.WorkspaceTest(t)` to get one.
## Tests
n/a
## Changes
Objectives:
* A dedicated directory for integration tests
* It is not picked up by `go test ./...`
* No need for a `TestAcc` test name prefix
* More granular packages to improve test selection (future)
The tree structure generally mirrors the source code tree structure.
Requirements for new files in this directory:
* Every package **must** be named after its directory with `_test` appended
* Requiring a different package name for integration tests avoids
aliasing with the main package.
* Every integration test package **must** include a `main_test.go` file.
These requirements are enforced by a unit test in the `integration` package.
## Tests
Integration tests pass.
The total run time regresses by about 10%. A follow-up change that
increases the degree of test parallelism will address this.
Shreyas Goenka
shreyas-goenka
Denis Bilenko
Pieter Noordhuis