[tango]: Add integration testing#156
Conversation
Add an integration test to tango. This test: * Spins up the tango server, configured with the tango repo itself as the target * Creates a client connected to it * Calls its APIs (GetTargetGraph and GetChangedTargets) * Asserts some responses from the server against expected data for actual commits in the tango repository. This provides a useful signal for how well tango works as an entire system. In a future PR, we can also re-use the same testing components to write benchmarks that measure tango's latency.
| t.Helper() | ||
| configPath := filepath.Join(dir, "tango-config.yaml") | ||
| content := fmt.Sprintf(`storage: | ||
| type: "memory" |
There was a problem hiding this comment.
let's make this a data to go_test and read from there
| func startServer(t *testing.T, remote string) string { | ||
| t.Helper() | ||
|
|
||
| cacheDir := filepath.Join(os.TempDir(), "tango-e2e-cache") |
There was a problem hiding this comment.
| cacheDir := filepath.Join(os.TempDir(), "tango-e2e-cache") | |
| cacheDir := filepath.Join(t.TempDir(), "tango-e2e-cache") |
| // 1f2e3e9 Honor OutputConfig include_hashes/include_tags/include_attributes (#116) | ||
| // The second commit adds controller/output_filter.go and output_filter_test.go. | ||
| const firstSHA = "046de2c20b5492cd5606d32fd632a38b8b70c8f6" | ||
| const secondSHA = "1f2e3e9245b159006cf2103becd51c5c1b6ec868" |
There was a problem hiding this comment.
can this be in Makefile envs instead?
| metadata = mergeMetadata(metadata, item.Metadata) | ||
| } | ||
| } | ||
| _ = stream.CloseSend() |
There was a problem hiding this comment.
let's defer this or make this a functional call. otherwise if require.NoError fails, this is a leak
func(){
defer steam.Closesend()
for {...}
}()
| require.NotEmpty(t, raw.metadata.GetTargetIdMapping()) | ||
| require.NotEmpty(t, raw.metadata.GetRuleTypeMapping()) | ||
|
|
||
| assert.Equal(t, 3673, len(raw.targets), "expected exact target count for pinned SHA") |
There was a problem hiding this comment.
this might be a bit fragile, cause if we upgrade bazel query with some options or change the hashing algorithm, the number can change.
How about only check a few known targets and their dependency counts?
There was a problem hiding this comment.
Makes sense, happy to change it if we don't think it's valuable. I was thinking of it as a sort of "golden"-like assertion. As in, even if it gets modified frequently, I'm wondering if it's still useful to have as a heads up? Is there value in knowing/being "forced" to investigate (by means of this test failing) how graph size changes with updates we make to tango? What do you think?
2 participants
Add some integration testing to tango. These tests:
I decided to use the actual tango repository to provide a heightened realism to the tests.
I also prototype a setup that generated an entire repository with multiple commits at runtime,
but just the boilerplate to bring in rules_go/other basics to support any non-contrived examples
seems to complicate the test code enough that I figured I might as well just use tango itself.
The downside of this approach is that I couldn't easily really assert against any actual piece of the graph
in entirety in testing GetTargetGraph, because it's pretty huge in a real repository with external dependencies. Instead, I assert against some specific expected edges.
This provides a useful signal for how well tango works as an entire system. In a future PR, we can also re-use the same testing components to write benchmarks that measure tango's latency.
This also adds a separate make target and CI step to run this since, in local testing, it can take about a minute or so.