Files
grafana/pkg/build/cmd/verifystarlark_test.go
T
Jack Baldry 8379a5338c CI: Lint starlark files with buildifier (#59157)
* Add verify-starlark build action that returns an error for starlark files with lint

Relies on `buildifier` tool.

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Add verify_starlark_step to PR pipeline

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Manually fetch buildifier in curl_image until a new build_image is created

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Format with buildifier

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Remove all unused variables retaining one unused function

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Use snake_case for variable

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Replace deprecated dictionary concatenation with .update() method

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Start adding docstrings for all modules and functions

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Prefer os.WriteFile as ioutil.WriteFile has been deprecated since go 1.16

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Attempt to document the behavior of the init_enterprise_step

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Document test_backend pipeline

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Document enterprise_downstream_step

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Document the pipeline utility function

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Document publish_images_step

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Document publish_images_steps

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Document enterprise2_pipelines function

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Add tags table for Starlark files.

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Document test_frontend

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Document windows function

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Add docstrings to verifystarlark functions

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Refactor error handling to be more clear and document complex behavior

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Split errors into execution errors and verification errors

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Document all other library functions

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Add local variables to TAGS

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Add blank line between all Args and Returns sections

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Fix new linting errors

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Lint new Starlark files

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Correct buildifier binary mv

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Document the need to set nofile ulimit to at least 2048

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Update build-container to include buildifier

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Ensure buildifier binary is executable

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Fix valid content test

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Simply return execution error

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Only check files rather than fixing them

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Use updated build-container with executable buildifier

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Test that context cancellation stops execution

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Simplify error handling

Return execution errors that short circuit WalkDir rather than
separately tracking that error.

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Remove fetching of buildifier binary now that it is in the build-container

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Use build image in verify-starlark step

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Use semver tag

The image is the same but uses a semver tag to make it clearer that
this is a forward upgrade from the old version.

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

* Use node 18 image with buildifier

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>

---------

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>
2023-01-30 09:27:11 +00:00

136 lines
4.8 KiB
Go

package main
import (
"context"
"os"
"path/filepath"
"strings"
"testing"
"github.com/stretchr/testify/require"
)
func TestVerifyStarlark(t *testing.T) {
t.Run("execution errors", func(t *testing.T) {
t.Run("invalid usage", func(t *testing.T) {
ctx := context.Background()
workspace := t.TempDir()
err := os.WriteFile(filepath.Join(workspace, "ignored.star"), []byte{}, os.ModePerm)
if err != nil {
t.Fatalf(err.Error())
}
_, executionErr := verifyStarlark(ctx, workspace, func(string) (string, []string) { return "buildifier", []string{"--invalid"} })
if executionErr == nil {
t.Fatalf("Expected execution error but got none")
}
})
t.Run("context cancellation", func(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
workspace := t.TempDir()
err := os.WriteFile(filepath.Join(workspace, "ignored.star"), []byte{}, os.ModePerm)
if err != nil {
t.Fatalf(err.Error())
}
err = os.WriteFile(filepath.Join(workspace, "other-ignored.star"), []byte{}, os.ModePerm)
if err != nil {
t.Fatalf(err.Error())
}
cancel()
_, executionErr := verifyStarlark(ctx, workspace, buildifierLintCommand)
if executionErr == nil {
t.Fatalf("Expected execution error but got none")
}
})
})
t.Run("verification errors", func(t *testing.T) {
t.Run("a single file with lint", func(t *testing.T) {
ctx := context.Background()
workspace := t.TempDir()
invalidContent := []byte(`load("scripts/drone/other.star", "function")
function()`)
err := os.WriteFile(filepath.Join(workspace, "has-lint.star"), invalidContent, os.ModePerm)
if err != nil {
t.Fatalf(err.Error())
}
verificationErrs, executionErr := verifyStarlark(ctx, workspace, buildifierLintCommand)
if executionErr != nil {
t.Fatalf("Unexpected execution error: %v", executionErr)
}
if len(verificationErrs) == 0 {
t.Fatalf(`"has-lint.star" requires linting but the verifyStarlark function provided no linting error`)
}
if len(verificationErrs) > 1 {
t.Fatalf(`verifyStarlark returned multiple errors for the "has-lint.star" file but only one was expected: %v`, verificationErrs)
}
if !strings.Contains(verificationErrs[0].Error(), "has-lint.star:1: module-docstring: The file has no module docstring.") {
t.Fatalf(`"has-lint.star" is missing a module docstring but the verifyStarlark function linting error did not mention this, instead we got: %v`, verificationErrs[0])
}
})
t.Run("no files with lint", func(t *testing.T) {
ctx := context.Background()
workspace := t.TempDir()
content := []byte(`"""
This module does nothing.
"""
load("scripts/drone/other.star", "function")
function()
`)
require.NoError(t, os.WriteFile(filepath.Join(workspace, "no-lint.star"), content, os.ModePerm))
verificationErrs, executionErr := verifyStarlark(ctx, workspace, buildifierLintCommand)
if executionErr != nil {
t.Fatalf("Unexpected execution error: %v", executionErr)
}
if len(verificationErrs) != 0 {
t.Log(`"no-lint.star" has no lint but the verifyStarlark function provided at least one error`)
for _, err := range verificationErrs {
t.Log(err)
}
t.FailNow()
}
})
t.Run("multiple files with lint", func(t *testing.T) {
ctx := context.Background()
workspace := t.TempDir()
invalidContent := []byte(`load("scripts/drone/other.star", "function")
function()`)
require.NoError(t, os.WriteFile(filepath.Join(workspace, "has-lint.star"), invalidContent, os.ModePerm))
require.NoError(t, os.WriteFile(filepath.Join(workspace, "has-lint2.star"), invalidContent, os.ModePerm))
verificationErrs, executionErr := verifyStarlark(ctx, workspace, buildifierLintCommand)
if executionErr != nil {
t.Fatalf("Unexpected execution error: %v", executionErr)
}
if len(verificationErrs) == 0 {
t.Fatalf(`Two files require linting but the verifyStarlark function provided no linting error`)
}
if len(verificationErrs) == 1 {
t.Fatalf(`Two files require linting but the verifyStarlark function provided only one linting error: %v`, verificationErrs[0])
}
if len(verificationErrs) > 2 {
t.Fatalf(`verifyStarlark returned more errors than expected: %v`, verificationErrs)
}
if !strings.Contains(verificationErrs[0].Error(), "has-lint.star:1: module-docstring: The file has no module docstring.") {
t.Errorf(`"has-lint.star" is missing a module docstring but the verifyStarlark function linting error did not mention this, instead we got: %v`, verificationErrs[0])
}
if !strings.Contains(verificationErrs[1].Error(), "has-lint2.star:1: module-docstring: The file has no module docstring.") {
t.Fatalf(`"has-lint2.star" is missing a module docstring but the verifyStarlark function linting error did not mention this, instead we got: %v`, verificationErrs[0])
}
})
})
}