Skip to content

Conversation

@jba
Copy link
Contributor

@jba jba commented Jun 16, 2024

Add a test designed to be run in different languages.

This uses a cross-language test framework I'm developing,
github.com/jba/xltest.

At its heart is the file testdata/googleai-embedder.yaml, which defines
the test in a language-neutral way. The test refers to a json file
with the expected trace.

The code in tests/googleai_test.go reads the test file and runs
the test. The main challenge is making two traces comparable,
which requires canonicalizing their span IDs.

To make sure we are testing the complete logic of an embedder action,
we pass a mock HTTP client to the googleai plugin, so we control
the interaction with the service.

Capturing traces cleanly also required some changes to the tracing
system, to allow installing a trace store temporarily and then
removing it.

Add a test designed to be run in different languages.

This uses a cross-language test framework I'm developing,
github.com/jba/xltest.

At its heart is the file testdata/googleai-embedder.yaml, which defines
the test in a language-neutral way. The test refers to a json file
with the expected trace.

The code in tests/googleai_test.go reads the test file and runs
the test. The main challenge is making two traces comparable,
which requires canonicalizing their span IDs.

To make sure we are testing the complete logic of an embedder action,
we pass a mock HTTP client to the googleai plugin, so we control
the interaction with the service.

Capturing traces cleanly also required some changes to the tracing
system, to allow installing a trace store temporarily and then
removing it.
@jba jba requested a review from MaesterChestnut June 18, 2024 07:34
@jba jba mentioned this pull request Jun 21, 2024
1 task
@@ -0,0 +1,36 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the source of this trace? This does not match with what is generated by the JS SDK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

YAY! That is exactly why we need these.
The source is the Go SDK.
Please replace with what is generated by JS so the Go can match it.
Also, please implement the similar test for JS so if the JS changes, your test will break too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Updated the trace here -- this test now fails.

Comment on lines +105 to +142
func renameSpans(td *tracing.Data) {
type item struct {
id string
t tracing.Milliseconds
}

var items []item
startTimes := map[tracing.Milliseconds]bool{}
for id, span := range td.Spans {
if startTimes[span.StartTime] {
panic("duplicate start times")
}
startTimes[span.StartTime] = true
items = append(items, item{id, span.StartTime})
}
slices.SortFunc(items, func(i1, i2 item) int {
return cmp.Compare(i1.t, i2.t)
})
oldIDToNew := map[string]string{}
for i, item := range items {
oldIDToNew[item.id] = fmt.Sprintf("s%03d", i)
}
// Re-create the span map with the new span IDs.
m := map[string]*tracing.SpanData{}
for oldID, span := range td.Spans {
newID := oldIDToNew[oldID]
if newID == "" {
panic(fmt.Sprintf("missing id: %q", oldID))
}
m[newID] = span
// A span references it own span ID and possibly its parent's.
span.SpanID = oldIDToNew[span.SpanID]
if span.ParentSpanID != "" {
span.ParentSpanID = oldIDToNew[span.ParentSpanID]
}
}
td.Spans = m
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pavelgj This code makes it possible to compare two traces completely, except for times and the trace ID.

@jba
Copy link
Contributor Author

jba commented Jul 11, 2024

I'm going to close this in favor of using the existing JS test framework for the reflection API. See #549 and #551.

This test did reveal some incompatibilities. One was that the Go embedder design didn't match the JS. #537 fixed that.

Others are that the display name doesn't see to be set in Go, and the "genkit:path" metadata values are very different. I don't know how important those are.
@pavelgj

@jba jba closed this Jul 11, 2024
@ssbushi ssbushi deleted the jba-xltest branch June 2, 2025 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants