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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ dist/

*.log
coverage.txt
coverage-acceptance.txt

__pycache__
*.pyc
Expand Down
13 changes: 12 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@ cover:
showcover:
go tool cover -html=coverage.txt

acc-cover:
rm -fr ./acceptance/build/cover/
CLI_GOCOVERDIR=build/cover go test ./acceptance
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.


acc-showcover:
go tool cover -html=coverage-acceptance.txt

build: vendor
go build -mod vendor

Expand All @@ -45,4 +56,4 @@ integration:
integration-short:
$(INTEGRATION) -short

.PHONY: lint lintcheck fmt test cover showcover build snapshot vendor schema integration integration-short
.PHONY: lint lintcheck fmt test cover showcover build snapshot vendor schema integration integration-short acc-cover acc-showcover
30 changes: 26 additions & 4 deletions acceptance/acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,16 @@ func TestAccept(t *testing.T) {
cwd, err := os.Getwd()
require.NoError(t, err)

execPath := BuildCLI(t, cwd)
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.

coverDir, err = filepath.Abs(coverDir)
require.NoError(t, err)
t.Logf("Writing coverage to %s", coverDir)
}

execPath := BuildCLI(t, cwd, coverDir)
// $CLI is what test scripts are using
t.Setenv("CLI", execPath)

Expand All @@ -60,10 +69,11 @@ func TestAccept(t *testing.T) {

testDirs := getTests(t)
require.NotEmpty(t, testDirs)

for _, dir := range testDirs {
t.Run(dir, func(t *testing.T) {
t.Parallel()
runTest(t, dir, repls)
runTest(t, dir, coverDir, repls)
})
}
}
Expand All @@ -88,7 +98,7 @@ func getTests(t *testing.T) []string {
return testDirs
}

func runTest(t *testing.T, dir string, repls testdiff.ReplacementsContext) {
func runTest(t *testing.T, dir, coverDir string, repls testdiff.ReplacementsContext) {
var tmpDir string
var err error
if KeepTmp {
Expand All @@ -111,6 +121,15 @@ func runTest(t *testing.T, dir string, repls testdiff.ReplacementsContext) {

args := []string{"bash", "-euo", "pipefail", EntryPointScript}
cmd := exec.Command(args[0], args[1:]...)
if coverDir != "" {
// Creating individual coverage directory for each test, because writing to the same one
// results in sporadic failures like this one (only if tests are running in parallel):
// +error: coverage meta-data emit failed: writing ... rename .../tmp.covmeta.b3f... .../covmeta.b3f2c...: no such file or directory
coverDir = filepath.Join(coverDir, strings.ReplaceAll(dir, string(os.PathSeparator), "--"))
err := os.MkdirAll(coverDir, os.ModePerm)
require.NoError(t, err)
cmd.Env = append(os.Environ(), "GOCOVERDIR="+coverDir)
}
cmd.Dir = tmpDir
outB, err := cmd.CombinedOutput()

Expand Down Expand Up @@ -210,14 +229,17 @@ func readMergedScriptContents(t *testing.T, dir string) string {
return strings.Join(prepares, "\n")
}

func BuildCLI(t *testing.T, cwd string) string {
func BuildCLI(t *testing.T, cwd, coverDir string) string {
execPath := filepath.Join(cwd, "build", "databricks")
if runtime.GOOS == "windows" {
execPath += ".exe"
}

start := time.Now()
args := []string{"go", "build", "-mod", "vendor", "-o", execPath}
if coverDir != "" {
args = append(args, "-cover")
}
cmd := exec.Command(args[0], args[1:]...)
cmd.Dir = ".."
out, err := cmd.CombinedOutput()
Expand Down
Loading