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

Coverage for acceptance tests #2123

Merged
merged 1 commit into from
Jan 14, 2025
Merged

Coverage for acceptance tests #2123

merged 1 commit into from
Jan 14, 2025

Conversation

denik
Copy link
Contributor

@denik denik commented Jan 13, 2025

Changes

Add two new make commands:

  • make acc-cover: runs acceptance tests and outputs coverage-acceptance.txt
  • make acc-showcover: show coverage-acceptance.txt locally in browser

Using the GOCOVERDIR functionality: https://go.dev/blog/integration-test-coverage

This works, but there are a couple of issues encountered:

  • GOCOVERDIR does not play well with regular "go test -cover". Once this fixed, we can simplify the code and have 'make cover' output coverage for everything at once. We can also probably get rid of CLI_GOCOVERDIR. runtime/coverage: confusing go test -cover + GOCOVERDIR behavior golang/go#66225
  • When running tests in parallel to the same directory there is rare conflict on writing covmeta file. For this reason each tests writes coverage to their own directory which is then merged together by 'make acc-cover'.

make acc-cover: runs acceptance tests and outputs coverage-acceptance.txt
make acc-showcover: show coverage-acceptance.txt locally in browser
rm -fr ./acceptance/build/cover-merged/
mkdir -p acceptance/build/cover-merged/
go tool covdata merge -i $$(printf '%s,' acceptance/build/cover/* | sed 's/,$$//') -o acceptance/build/cover-merged/
go tool covdata textfmt -i acceptance/build/cover-merged -o coverage-acceptance.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be implemented in Go so we run go test ./acceptance -coverprofile=coverage-acceptance.txt (see gotestsum line above for reference). The path manipulation + expansion + sed here feels fragile.

Can be a possible (not required) follow-up.

coverDir := os.Getenv("CLI_GOCOVERDIR")

if coverDir != "" {
require.NoError(t, os.MkdirAll(coverDir, os.ModePerm))
Copy link
Contributor

Choose a reason for hiding this comment

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

os.ModePerm expands to 0o777 -- intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, does not matter here.

@denik denik added this pull request to the merge queue Jan 14, 2025
Merged via the queue into main with commit a5e09ab Jan 14, 2025
9 checks passed
@denik denik deleted the denik/acceptance-coverage branch January 14, 2025 14:25
andrewnester added a commit that referenced this pull request Jan 16, 2025
New feature announcement.

You can now manage Databricks Apps using DABs by defining an `app` resource in your bundle configuration.
For more information see Databricks documentation https://docs.databricks.com/en/dev-tools/bundles/resources.html#app

CLI:
 * Filter out system clusters in cluster picker ([#2131](#2131)).
 * Process all the fields in top level request object even if it contains request body ([#2155](#2155)).

Bundles:
 * Added support for Databricks Apps in DABs ([#1928](#1928)).
 * Allow artifact path to be located outside the sync root ([#2128](#2128)).
 * Retry app deployment if there is an active deployment in progress ([#2153](#2153)).
 * Resolve variables in a loop ([#2164](#2164)).
 * Improve resolution of complex variables within complex variables ([#2157](#2157)).
 * Added output message to warn about slower deployments with apps ([#2161](#2161)).
 * Patch references to UC schemas to capture dependencies automatically ([#1989](#1989)).
 * Format default-python template ([#2110](#2110)).
 * Encourage the use of root_path in production to ensure single deployment ([#1712](#1712)).
 * Log warnings to stderr for "bundle validate -o json" ([#2109](#2109)).

Internal:
 * Move merge fix-ups after variable resolution ([#2125](#2125)).
 * Enable linter 'unconvert' and fix the issues found ([#2136](#2136)).
 * Coverage for acceptance tests ([#2123](#2123)).
 * Add acceptance tests for builtin templates ([#2135](#2135)).
 * Add a unique schema for recreate pipeline test ([#2159](#2159)).
 * Migrate resolution tests to acceptance tests ([#2143](#2143)).
 * Update runner for the publish-winget job ([#2105](#2105)).
 * Add a test for complex variable resolution with 3 levels ([#2163](#2163)).

API Changes:
 * Changed `databricks account federation-policy update` command with new required argument order.
 * Changed `databricks account service-principal-federation-policy update` command with new required argument order.

OpenAPI commit 779817ed8d63031f5ea761fbd25ee84f38feec0d (2025-01-08)
Dependency updates:
 * Upgrade TF provider to 1.63.0 ([#2162](#2162)).
 * Bump golangci-lint version to v1.63.4 from v1.63.1 ([#2114](#2114)).
 * Bump astral-sh/setup-uv from 4 to 5 ([#2116](#2116)).
 * Bump golang.org/x/oauth2 from 0.24.0 to 0.25.0 ([#2080](#2080)).
 * Bump github.com/hashicorp/hc-install from 0.9.0 to 0.9.1 ([#2079](#2079)).
 * Bump golang.org/x/term from 0.27.0 to 0.28.0 ([#2078](#2078)).
 * Bump github.com/databricks/databricks-sdk-go from 0.54.0 to 0.55.0 ([#2126](#2126)).
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.

2 participants