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.

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

magefile: added combined test coverage support for mage test:all #2235

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+135 −66
Open

magefile: added combined test coverage support for mage test:all #2235

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
80503bb
added test coverage
kartikaysaxena Feb 4, 2025
64f1377
different profiles for tests
kartikaysaxena Feb 5, 2025
3c14817
Merge branch 'main' into mage-test-coverage
kartikaysaxena Feb 8, 2025
59fd80d
use path hash instead
kartikaysaxena Feb 8, 2025
3f62b00
separate targets for cover
kartikaysaxena Feb 10, 2025
752a5ea
context coverage bool
kartikaysaxena Feb 13, 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
287 changes: 227 additions & 60 deletions magefiles/test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,23 @@ func (t Test) All() error {
return nil
}

// All Runs all test suites and generates a combined coverage report
func (t Test) AllCover() error {
ds := Testds{}
c := Testcons{}
mg.Deps(t.UnitCover, t.IntegrationCover, t.SteelthreadCover, t.ImageCover, t.AnalyzersCover,
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems like a lot of duplication that would lead to drift w.r.t to All() method. Ideally All and AllCover invoke `all(coverage bool)

ds.CrdbCover, ds.PostgresCover, ds.SpannerCover, ds.MysqlCover,
c.CrdbCover, c.SpannerCover, c.PostgresCover, c.MysqlCover)
return combineCoverage()
}

// UnitCover Runs the unit tests and generates a coverage report
func (t Test) UnitCover() error {
if err := t.unit(true); err != nil {
return err
}
fmt.Println("Running coverage...")
return sh.RunV("go", "tool", "cover", "-html=coverage.txt")
return nil
}

// Unit Runs the unit tests
Expand All @@ -43,27 +53,75 @@ func (Test) unit(coverage bool) error {
fmt.Println("running unit tests")
args := []string{"-tags", "ci,skipintegrationtests", "-race", "-timeout", "10m", "-count=1"}
if coverage {
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be moved into goTest, otherwise it leads to a lot of duplication

args = append(args, "-covermode=atomic", "-coverprofile=coverage.txt")
args = append(args, "-covermode=atomic", fmt.Sprintf("-coverprofile=coverage-%s.txt", hashPath("./...")))
}
return goTest("./...", args...)
}

// Image Run tests that run the built image and generates a coverage report
func (t Test) ImageCover() error {
if err := t.image(true); err != nil {
return err
}
return nil
}

// Image Run tests that run the built image
func (Test) Image() error {
mg.Deps(Build{}.Testimage)
return goDirTest("./cmd/spicedb", "./...", "-tags", "docker,image")
}

// Integration Run integration tests
func (Test) Integration() error {
mg.Deps(checkDocker)
return goTest("./internal/services/integrationtesting/...", "-tags", "ci,docker", "-timeout", "15m")
func (Test) image(coverage bool) error {
mg.Deps(Build{}.Testimage)
args := []string{"-tags", "docker,image"}
if coverage {
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be moved into goDirTest, otherwise it leads to a lot of duplication

args = append(args, "-covermode=atomic", fmt.Sprintf("-coverprofile=coverage-%s.txt", hashPath("./...")))
}
return goDirTest("./cmd/spicedb", "./...", args...)
}

// Steelthread Run steelthread tests
func (Test) Steelthread() error {
// IntegrationCover Runs the integration tests and generates a coverage report
func (t Test) IntegrationCover() error {
if err := t.integration(true); err != nil {
return err
}
return nil
}

// Integration Runs the integration tests
func (t Test) Integration() error {
return t.integration(false)
}

func (Test) integration(coverage bool) error {
args := []string{"-tags", "ci,docker", "-timeout", "15m"}
if coverage {
args = append(args, "-covermode=atomic", fmt.Sprintf("-coverprofile=coverage-%s.txt", hashPath("./internal/services/integrationtesting/...")))
}
return goTest("./internal/services/integrationtesting/...", args...)
}

// SteelthreadCover Runs the steelthread tests and generates a coverage report
func (t Test) SteelthreadCover() error {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's annoying we end up with proliferation of commands just to have with/without coverage versions of them. Calling mage leads to a very long list that starts to become not very user friendly.

I know mage supports arguments in commands, but not if it supports commands with arguments and default values, could you investigate?

if err := t.steelthread(true); err != nil {
return err
}
return nil
}

// Steelthread Runs the steelthread tests
func (t Test) Steelthread() error {
return t.steelthread(false)
}

func (Test) steelthread(coverage bool) error {
fmt.Println("running steel thread tests")
return goTest("./internal/services/steelthreadtesting/...", "-tags", "steelthread,docker,image", "-timeout", "15m", "-v")
args := []string{"-tags", "steelthread,docker,image", "-timeout", "15m", "-v"}
if coverage {
args = append(args, "-covermode=atomic", fmt.Sprintf("-coverprofile=coverage-%s.txt", hashPath("./internal/services/steelthreadtesting/...")))
}
return goTest("./internal/services/steelthreadtesting/...", args...)
}

// RegenSteelthread Regenerate the steelthread tests
Expand All @@ -74,9 +132,25 @@ func (Test) RegenSteelthread() error {
}), WithArgs("test", "./internal/services/steelthreadtesting/...", "-tags", "steelthread,docker,image", "-timeout", "15m", "-v"))("go")
}

// AnalyzersCover Runs the analyzer tests and generates a coverage report
func (t Test) AnalyzersCover() error {
if err := t.analyzers(true); err != nil {
return err
}
return nil
}

// Analyzers Run the analyzer unit tests
func (Test) Analyzers() error {
return goDirTest("./tools/analyzers", "./...")
func (t Test) Analyzers() error {
return t.analyzers(false)
}

func (Test) analyzers(coverage bool) error {
args := []string{}
if coverage {
args = append(args, "-covermode=atomic", fmt.Sprintf("-coverprofile=coverage-%s.txt", hashPath("./...")))
}
return goDirTest("./tools/analyzers", "./...", args...)
}

// Wasm Run wasm browser tests
Expand All @@ -94,39 +168,96 @@ func (Test) Wasm() error {

type Testds mg.Namespace

// Crdb Run datastore tests for crdb
func (tds Testds) Crdb() error {
return tds.crdb("")
// CrdbCover Runs the CRDB datastore tests and generates a coverage report
func (tds Testds) CrdbCover() error {
if err := tds.crdb(true, ""); err != nil {
return err
}
return nil
}

func (tds Testds) CrdbVer(version string) error {
return tds.crdb(version)
// Crdb Runs the CRDB datastore tests
func (tds Testds) Crdb() error {
return tds.crdb(false, "")
}

func (Testds) crdb(version string) error {
func (Testds) crdb(coverage bool, version string) error {
args := []string{"-tags", "ci,docker", "-timeout", "10m"}
if coverage {
args = append(args, "-covermode=atomic", fmt.Sprintf("-coverprofile=coverage-%s.txt", hashPath("./internal/datastore/crdb/...")))
}
return datastoreTest("crdb", map[string]string{
"CRDB_TEST_VERSION": version,
})
}, args...)
}

// Spanner Run datastore tests for spanner
func (Testds) Spanner() error {
return datastoreTest("spanner", emptyEnv)
// PostgresCover Runs the Postgres datastore tests and generates a coverage report
func (tds Testds) PostgresCover() error {
if err := tds.postgres(true, ""); err != nil {
return err
}
return nil
}

// Postgres Run datastore tests for postgres
// Postgres Runs the Postgres datastore tests
func (tds Testds) Postgres() error {
return tds.postgres("")
return tds.postgres(false, "")
}

func (tds Testds) PostgresVer(version string) error {
return tds.postgres(version)
}

func (Testds) postgres(version string) error {
func (Testds) postgres(coverage bool, version string) error {
args := []string{"-tags", "ci,docker", "-timeout", "10m"}
if coverage {
args = append(args, "-covermode=atomic", fmt.Sprintf("-coverprofile=coverage-%s.txt", hashPath("./internal/datastore/postgres/...")))
}
return datastoreTest("postgres", map[string]string{
"POSTGRES_TEST_VERSION": version,
}, "postgres")
}, args...)
}

// SpannerCover Runs the Spanner datastore tests and generates a coverage report
func (tds Testds) SpannerCover() error {
if err := tds.spanner(true); err != nil {
return err
}
return nil
}

// Spanner Runs the Spanner datastore tests
func (tds Testds) Spanner() error {
return tds.spanner(false)
}

func (Testds) spanner(coverage bool) error {
args := []string{"-tags", "ci,docker", "-timeout", "10m"}
if coverage {
Copy link
Contributor

Choose a reason for hiding this comment

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

lots of duplication: this coverage if statement should be handled inside datastoreTest

args = append(args, "-covermode=atomic", fmt.Sprintf("-coverprofile=coverage-%s.txt", hashPath("./internal/datastore/spanner/...")))
}
return datastoreTest("spanner", emptyEnv, args...)
}

// MysqlCover Runs the MySQL datastore tests and generates a coverage report
func (tds Testds) MysqlCover() error {
if err := tds.mysql(true); err != nil {
return err
}
return nil
}

// Mysql Runs the MySQL datastore tests
func (tds Testds) Mysql() error {
return tds.mysql(false)
}

func (Testds) mysql(coverage bool) error {
args := []string{"-tags", "ci,docker", "-timeout", "10m"}
if coverage {
args = append(args, "-covermode=atomic", fmt.Sprintf("-coverprofile=coverage-%s.txt", hashPath("./internal/datastore/mysql/...")))
}
return datastoreTest("mysql", emptyEnv, args...)
}

func (tds Testds) PostgresVer(version string) error {
return tds.postgres(false, version)
}

// Pgbouncer Run datastore tests for postgres with Pgbouncer
Expand All @@ -144,11 +275,6 @@ func (Testds) pgbouncer(version string) error {
}, "pgbouncer")
}

// Mysql Run datastore tests for mysql
func (Testds) Mysql() error {
return datastoreTest("mysql", emptyEnv)
}

func datastoreTest(datastore string, env map[string]string, tags ...string) error {
mergedTags := append([]string{"ci", "docker"}, tags...)
tagString := strings.Join(mergedTags, ",")
Expand All @@ -158,38 +284,84 @@ func datastoreTest(datastore string, env map[string]string, tags ...string) erro

type Testcons mg.Namespace

// Crdb Run consistency tests for crdb
// CrdbCover Runs the CRDB consistency tests and generates a coverage report
func (tc Testcons) CrdbCover() error {
if err := tc.crdb(true, ""); err != nil {
return err
}
return nil
}

// Crdb Runs the CRDB consistency tests
func (tc Testcons) Crdb() error {
return tc.crdb("")
return tc.crdb(false, "")
}

func (tc Testcons) CrdbVer(version string) error {
return tc.crdb(version)
return tc.crdb(false, version)
}

func (Testcons) crdb(version string) error {
func (Testcons) crdb(coverage bool, version string) error {
return consistencyTest("crdb", map[string]string{
"CRDB_TEST_VERSION": version,
})
}, coverage)
}

// Spanner Run consistency tests for spanner
func (Testcons) Spanner() error {
return consistencyTest("spanner", emptyEnv)
// PostgresCover Runs the Postgres consistency tests and generates a coverage report
func (tc Testcons) PostgresCover() error {
if err := tc.postgres(true, ""); err != nil {
return err
}
return nil
}

// Postgres Runs the Postgres consistency tests
func (tc Testcons) Postgres() error {
return tc.postgres("")
}

func (tc Testcons) PostgresVer(version string) error {
return tc.postgres(version)
return tc.postgres(false, "")
}

func (Testcons) postgres(version string) error {
func (Testcons) postgres(coverage bool, version string) error {
return consistencyTest("postgres", map[string]string{
"POSTGRES_TEST_VERSION": version,
})
}, coverage)
}

// SpannerCover Runs the Spanner consistency tests and generates a coverage report
func (tc Testcons) SpannerCover() error {
if err := tc.spanner(true); err != nil {
return err
}
return nil
}

// Spanner Runs the Spanner consistency tests
func (tc Testcons) Spanner() error {
return tc.spanner(false)
}

func (Testcons) spanner(coverage bool) error {
return consistencyTest("spanner", emptyEnv, coverage)
}

// MysqlCover Runs the MySQL consistency tests and generates a coverage report
func (tc Testcons) MysqlCover() error {
if err := tc.mysql(true); err != nil {
return err
}
return nil
}

// Mysql Runs the MySQL consistency tests
func (tc Testcons) Mysql() error {
return tc.mysql(false)
}

func (Testcons) mysql(coverage bool) error {
return consistencyTest("mysql", emptyEnv, coverage)
}

func (tc Testcons) PostgresVer(version string) error {
return tc.postgres(false, version)
}

// Pgbouncer Run consistency tests for postgres with pgbouncer
Expand All @@ -204,16 +376,11 @@ func (Testcons) PgbouncerVer(version string) error {
return nil
}

// Mysql Run consistency tests for mysql
func (Testcons) Mysql() error {
return consistencyTest("mysql", emptyEnv)
}

func consistencyTest(datastore string, env map[string]string) error {
func consistencyTest(datastore string, env map[string]string, coverage bool) error {
args := []string{"-tags", "ci,docker,datastoreconsistency", "-timeout", "10m", "-run", fmt.Sprintf("TestConsistencyPerDatastore/%s", datastore)}
mg.Deps(checkDocker)
return goDirTestWithEnv(".", "./internal/services/integrationtesting/...",
env,
"-tags", "ci,docker,datastoreconsistency",
"-timeout", "10m",
"-run", fmt.Sprintf("TestConsistencyPerDatastore/%s", datastore))
if coverage {
Copy link
Contributor

Choose a reason for hiding this comment

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

all this if coverage blocks could be refactored to avoid all the duplication - e.g. a function that takes the args and the coverage bool, and returns the args with the appended parameters.

args = append(args, "-covermode=atomic", fmt.Sprintf("-coverprofile=coverage-%s.txt", hashPath("./internal/services/integrationtesting/...")))
}
return goDirTestWithEnv(".", "./internal/services/integrationtesting/...", env, args...)
}
Loading