[DDO-3280] TestData mechanism

[jack-r-warren]

Test-only PR, no risk. I'm going to spew a bunch of explanation here about why I think this is so cool.

The Problem

We want to have some sort of example data available for use in tests. This helps make tests less verbose -- without it, for example, creating a chart release requires also creating a chart, environment, and cluster. The repetition of doing so throughout the codebase makes refactoring or adding validation more difficult, too.

What Sherlock v2 Did

Inside test files, Sherlock's v2 controllers would define vars with essentially API request bodies.

sherlock/internal/deprecated_controllers/v2controllers/chart_test.go

var leonardoChart = CreatableChart{
		Name: "leonardo",
		EditableChart: EditableChart{
			AppImageGitRepo:       utils.PointerTo("DataBiosphere/leonardo"),
			AppImageGitMainBranch: utils.PointerTo("main"),
			ChartExposesEndpoint:  utils.PointerTo(true),
			LegacyConfigsEnabled:  utils.PointerTo(true),
		},
	}

There'd also be a var list of all the request bodies, and a method to lob them all at Sherlock to add them all to the database.

sherlock/internal/deprecated_controllers/v2controllers/chart_test.go

func (controllerSet *ControllerSet) seedCharts(t *testing.T, db *gorm.DB) {
	for _, creatable := range chartSeedList {
		if _, _, err := controllerSet.ChartController.Create(creatable, generateUser(t, db, false)); err != nil {
			t.Errorf("error seeding chart %s: %v", creatable.Name, err)
		}
	}
}

In a test, you'd need to call suite.seedCharts(suite.T(), suite.db), then if you query the database it won't be empty anymore.

The Problem With What Sherlock v2 Did

The issue rears its head as soon as we get into relational types. Take what we do for chart versions:

sherlock/internal/deprecated_controllers/v2controllers/chart_version_test.go

var leonardo1ChartVersion = CreatableChartVersion{
		Chart:        leonardoChart.Name,
		ChartVersion: "1.2.3",
	}

The database defines relations in terms of IDs. We certainly don't know those IDs statically, so we're forced to define our test data at the API level, where we can define relations in terms of API-user-friendly things like the name. This means that our test data is only useful to us in one package! We'll get circular import errors if we try to bring this test data into other packages, since everything ends up being re-imported by the API package.

More importantly, though, this completely breaks down for types that don't have another way to reference them (types that only have IDs, like changesets or deploy hooks) or for anything that's more internal and doesn't have a creation endpoint.

How We Want To Fix It

For API tests we will always need to define some request payloads, but we can solve a lot of our problems if we can figure out how to define test data at a lower level. To do that and still be able to set up relations, we'll need to somehow reference IDs that we don't know at compile-time (in fact, they'll potentially be different on each test run).

The Solution

In Sherlock v3, the low-level models package already exports a TestSuiteHelper that is meant to be embedded side-by-side with the stretchr/testify library's Suite (example). That helper sets up and tears down the database connection and is model-aware to let tests impersonate a User (so the tests from any package that uses the helper can properly use the database).

What's new is that the TestSuiteHelper now includes TestData. TestData provides methods that return sample data, and those same methods handle making sure it's in the database.

For example, to get ahold of an example value for Leonardo's chart, you'd call TestData.Chart_Leonardo().

The underscores are non-standard for source code, I know, but they seem more allowed by convention in tests and I think they really do make it more readable here, since we can't nest the methods like TestData.Chart.Leonardo()

The trick is the testDataImpl struct. It holds a back-reference to the TestSuiteHelper (so it can call the database) and it also caches within the context of each test. You can call TestData.Chart_Leonardo() repeatedly and only the first call will hit the database. At the end of each test function, the TestSuiteHelper wipes away the testDataImpl to reset the cache as it wipes away the database.

Here's TestData.Chart_Leonardo():

func (td *testDataImpl) Chart_Leonardo() Chart {
	if td.chart_leonardo.ID == 0 {
		td.chart_leonardo = Chart{
			Name:                  "leonardo",
			ChartRepo:             utils.PointerTo("terra-helm"),
			AppImageGitRepo:       utils.PointerTo("DataBiosphere/leonardo"),
			AppImageGitMainBranch: utils.PointerTo("main"),
			ChartExposesEndpoint:  utils.PointerTo(true),
			DefaultSubdomain:      utils.PointerTo("leonardo"),
			DefaultProtocol:       utils.PointerTo("https"),
			DefaultPort:           utils.PointerTo[uint](443),
		}
		td.create(&td.chart_leonardo)
	}
	return td.chart_leonardo
}

It's not quite as nice as a var or const, but here's the magic. Suppose we want a chart version. Here's TestData.ChartVersion_Leonardo_V1():

func (td *testDataImpl) ChartVersion_Leonardo_V1() ChartVersion {
	if td.chartVersion_leonardo_v1.ID == 0 {
		td.chartVersion_leonardo_v1 = ChartVersion{
			ChartID:      td.Chart_Leonardo().ID,
			ChartVersion: "0.1.0",
		}
		td.h.SetSuitableTestUserForDB()
		td.create(&td.chartVersion_leonardo_v1)
	}
	return td.chartVersion_leonardo_v1
}

We can get the chart ID here! That call to td.Chart_Leonardo().ID will create the Leonardo chart if it doesn't exist, just in time.

Chart versions are a little special too -- they like to read the user that's creating them to store it in the AuthoredBy field. No big deal: we have a reference to the TestSuiteHelper here (h), so we can treat this like a test and set the user before creating the chart version. Many other data types do something similar but for validating permissions, so this capability is deceptively important.

But wait, how does TestSuiteHelper.SetSuitableTestUserForDB() work? Guess what, it just borrows TestData.User_Suitable(), it's all self-consistent (TestSuiteHelper.SetSuitableTestUserForDB() existed before this PR, so we're bringing it along for the ride):

func (h *TestSuiteHelper) SetSuitableTestUserForDB() *User {
	return h.SetUserForDB(utils.PointerTo(h.TestData.User_Suitable()))
}

The wonderful part about this is that it's lazy. We can add as much test data as we like without bogging down or changing any existing tests. In fact, this TestData is already available everywhere because it's in TestSuiteHelper, but all of our tests that expect an empty database etc. still work just fine. There's still certainly times where manually setting up data is nice (here's a good example) but this should make ongoing Sherlock development faster and easier.

Example

Here's an example included with this PR of using the test data and how it can eliminate dummy data and boilerplate:

https://github.com/broadinstitute/sherlock/pull/348/files#diff-2ae792b7dfecde5ad6aaa265cd4e8f594638019c59ec565df7e4c38fee3849c9R52

-	s.SetNonSuitableTestUserForDB()
-	chart := Chart{Name: "name", ChartRepo: utils.PointerTo("repo")}
-	s.NoError(s.DB.Create(&chart).Error)
-	chartVersion := ChartVersion{ChartID: chart.ID, ChartVersion: "version"}
-	s.NoError(s.DB.Create(&chartVersion).Error)
+	chartVersion := s.TestData.ChartVersion_Leonardo_V1()

[github-actions]

No API changes detected

[github-actions]

Published image from 3c3a54a (merge b434b76):

us-central1-docker.pkg.dev/dsp-artifact-registry/sherlock/sherlock:v0.2.30-b434b76

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
5 Code Smells

No Coverage information
0.0% Duplication