Skip to content

Add GO_TESTFLAGS variable to append to test flags #352

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.

Sign up for GitHub

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

Merged
majewsky merged 7 commits into from
Sep 2, 2025
Merged

Add GO_TESTFLAGS variable to append to test flags #352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f828375
Add `GO_TESTFLAGS` variable to append to test flags
stanislav-zaprudskiy Aug 21, 2025
1ec8db1
Run go-makefile-maker
github-actions[bot] Aug 21, 2025
d982b7d
Fixup: make order of appearance of `GO_TEST*` variables in README mor…
stanislav-zaprudskiy Aug 26, 2025
dd4c2f5
Update all variables to support append and override
stanislav-zaprudskiy Aug 26, 2025
bec3c18
Update README to expand on variables override behavior
stanislav-zaprudskiy Aug 26, 2025
2beaa83
Run go-makefile-maker
github-actions[bot] Aug 26, 2025
a7203a0
Merge branch 'main' into add_test_args
majewsky Sep 1, 2025
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
12 changes: 7 additions & 5 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,11 @@ install-reuse: FORCE

prepare-static-check: FORCE install-golangci-lint install-modernize install-shellcheck install-go-licence-detector install-addlicense install-reuse

GO_BUILDFLAGS = -mod vendor
GO_LDFLAGS =
GO_TESTENV =
GO_BUILDENV =
GO_BUILDFLAGS := -mod vendor $(GO_BUILDFLAGS)
GO_LDFLAGS := $(GO_LDFLAGS)
GO_TESTFLAGS := $(GO_TESTFLAGS)
GO_TESTENV := $(GO_TESTENV)
GO_BUILDENV := $(GO_BUILDENV)

# These definitions are overridable, e.g. to provide fixed version/commit values when
# no .git directory is present or to provide a fixed build date for reproducibility.
Expand Down Expand Up @@ -107,7 +108,7 @@ run-shellcheck: FORCE install-shellcheck

build/cover.out: FORCE | build
@printf "\e[1;36m>> Running tests\e[0m\n"
@env $(GO_TESTENV) go test -shuffle=on -p 1 -coverprofile=build/coverprofile.out $(GO_BUILDFLAGS) -ldflags '-s -w -X github.com/sapcc/go-api-declarations/bininfo.binName=go-makefile-maker -X github.com/sapcc/go-api-declarations/bininfo.version=$(BININFO_VERSION) -X github.com/sapcc/go-api-declarations/bininfo.commit=$(BININFO_COMMIT_HASH) -X github.com/sapcc/go-api-declarations/bininfo.buildDate=$(BININFO_BUILD_DATE) $(GO_LDFLAGS)' -covermode=count -coverpkg=$(subst $(space),$(comma),$(GO_COVERPKGS)) $(GO_TESTPKGS)
@env $(GO_TESTENV) go test -shuffle=on -p 1 -coverprofile=build/coverprofile.out $(GO_BUILDFLAGS) -ldflags '-s -w -X github.com/sapcc/go-api-declarations/bininfo.binName=go-makefile-maker -X github.com/sapcc/go-api-declarations/bininfo.version=$(BININFO_VERSION) -X github.com/sapcc/go-api-declarations/bininfo.commit=$(BININFO_COMMIT_HASH) -X github.com/sapcc/go-api-declarations/bininfo.buildDate=$(BININFO_BUILD_DATE) $(GO_LDFLAGS)' -covermode=count -coverpkg=$(subst $(space),$(comma),$(GO_COVERPKGS)) $(GO_TESTFLAGS) $(GO_TESTPKGS)
@awk < build/coverprofile.out '$$1 != "mode:" { is_filename[$$1] = true; counts1[$$1]+=$$2; counts2[$$1]+=$$3 } END { for (filename in is_filename) { printf "%s %d %d\n", filename, counts1[filename], counts2[filename]; } }' | sort | $(SED) '1s/^/mode: count\n/' > $@

build/cover.html: build/cover.out
Expand Down Expand Up @@ -176,6 +177,7 @@ vars: FORCE
@printf "GO_COVERPKGS=$(GO_COVERPKGS)\n"
@printf "GO_LDFLAGS=$(GO_LDFLAGS)\n"
@printf "GO_TESTENV=$(GO_TESTENV)\n"
@printf "GO_TESTFLAGS=$(GO_TESTFLAGS)\n"
@printf "GO_TESTPKGS=$(GO_TESTPKGS)\n"
@printf "MAKE=$(MAKE)\n"
@printf "PREFIX=$(PREFIX)\n"
Expand Down
21 changes: 21 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,13 +447,28 @@ variables:
GO_BUILDFLAGS: '-mod vendor'
GO_LDFLAGS: ''
GO_TESTENV: ''
GO_TESTFLAGS: ''
```

Allows to override the default values of Makefile variables used by the autogenerated recipes.
This mechanism cannot be used to define new variables to use in your own rules; use `verbatim` for that.
By default, all accepted variables are empty.
The only exception is that `GO_BUILDFLAGS` defaults to `-mod vendor` when vendoring is enabled (see below).

When invoking the respective targets utilizing the variables, it's possible to either append to the provided values by modifying the calling environment:

```console
$ GO_BUILDFLAGS="-work -v" make build/go-makefile-maker
env go build -mod vendor -work -v -ldflags ...
```

or to override their values by passing the variables as `make` arguments:

```console
$ make build/go-makefile-maker GO_BUILDFLAGS="-mod=mod"
env go build -mod=mod -ldflags ...
```

A typical usage of `GO_LDFLAGS` is to give compile-time values to the Go compiler with the `-X` linker flag:

```yaml
Expand All @@ -472,6 +487,12 @@ variables:
GO_TESTENV: 'POSTGRES_HOST=localhost POSTGRES_DATABASE=unittestdb'
```

`GO_TESTFLAGS` can be used to provide additional flags to append to the test command flags:

```yaml
GO_TESTFLAGS: '-count=1 -short'
```

`GO_BUILDENV` can contain environment variables to pass to `go build`:

```yaml
Expand Down
11 changes: 6 additions & 5 deletions internal/makefile/makefile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,11 @@ endif
}

if isGolang {
build.addDefinition("GO_BUILDFLAGS =%s", cfg.Variable("GO_BUILDFLAGS", defaultBuildFlags))
build.addDefinition("GO_LDFLAGS =%s", cfg.Variable("GO_LDFLAGS", strings.TrimSpace(defaultLdFlags)))
build.addDefinition("GO_TESTENV =%s", cfg.Variable("GO_TESTENV", ""))
build.addDefinition("GO_BUILDENV =%s", cfg.Variable("GO_BUILDENV", ""))
build.addDefinition("GO_BUILDFLAGS :=%s $(GO_BUILDFLAGS)", cfg.Variable("GO_BUILDFLAGS", defaultBuildFlags))
build.addDefinition("GO_LDFLAGS :=%s $(GO_LDFLAGS)", cfg.Variable("GO_LDFLAGS", strings.TrimSpace(defaultLdFlags)))
build.addDefinition("GO_TESTFLAGS :=%s $(GO_TESTFLAGS)", cfg.Variable("GO_TESTFLAGS", ""))
Copy link
Copy Markdown
Contributor Author

@stanislav-zaprudskiy stanislav-zaprudskiy Aug 21, 2025

Choose a reason for hiding this comment

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

It's using slightly different approach here for the assignment to accommodate opportunities to both set and append to the variable.

Given the following configuration:

variables:
  GO_TESTFLAGS: '-short'
verbatim: |
  foo:
    go test $(GO_TESTFLAGS) pkg.example/foo

A normal target call would render:

❯ make foo
go test -short  pkg.example/foo

In order to append test flags one would do:

❯ GO_TESTFLAGS="-v -count=1" make foo
go test -short -v -count=1 pkg.example/foo

In order to override test flags one would do:

❯ make foo GO_TESTFLAGS=
go test  pkg.example/foo

(note, the new target foo is merely for representation purpose - the introduced variable is used in the main test command target).

If the proposed behavior is fine, I'd add a paragraph to describe it in README. It may be reasonable to also add the same for other variables - though this would backward incompatible with previous behavior, so I'm open for your suggestions.

Copy link
Copy Markdown
Contributor

@majewsky majewsky Aug 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I checked in my workgroup; we don't see a strong reason for why this should not be allowed. (Maybe this will blow up somewhere because it does break backwards compatibility, but we're willing to risk that.) Please apply this scheme to all variables: and document accordingly.

Copy link
Copy Markdown
Contributor Author

@stanislav-zaprudskiy stanislav-zaprudskiy Aug 26, 2025

Choose a reason for hiding this comment

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

Done in

build.addDefinition("GO_TESTENV :=%s $(GO_TESTENV)", cfg.Variable("GO_TESTENV", ""))
build.addDefinition("GO_BUILDENV :=%s $(GO_BUILDENV)", cfg.Variable("GO_BUILDENV", ""))
}
if sr.HasBinInfo {
build.addDefinition("")
Expand Down Expand Up @@ -383,7 +384,7 @@ endif
if sr.UseGinkgo {
testRunner = "go run github.com/onsi/ginkgo/v2/ginkgo run --randomize-all -output-dir=build"
}
goTest := fmt.Sprintf(`%s $(GO_BUILDFLAGS) -ldflags '%s $(GO_LDFLAGS)' -covermode=count -coverpkg=$(subst $(space),$(comma),$(GO_COVERPKGS)) $(GO_TESTPKGS)`,
goTest := fmt.Sprintf(`%s $(GO_BUILDFLAGS) -ldflags '%s $(GO_LDFLAGS)' -covermode=count -coverpkg=$(subst $(space),$(comma),$(GO_COVERPKGS)) $(GO_TESTFLAGS) $(GO_TESTPKGS)`,
testRunner, makeDefaultLinkerFlags(path.Base(sr.ModulePath), sr))
if runControllerGen {
testRule.prerequisites = append(testRule.prerequisites, "generate", "install-setup-envtest")
Expand Down
Loading