-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[Proposal] log/logtest: Redesign 2nd edition #6464
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.
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
Changes from all commits
e52793e
17d92c9
030f8a7
bf915ff
c77a09f
a470236
d8c692d
49486c6
c89f6c9
748c27d
96271e9
e6b7de6
578f9ab
9d68e80
e14db7f
e056b95
7443c45
6542559
a84ba46
a4c181d
69a1b37
c4ac836
1b0a436
178b0db
f26e543
f8b7763
59b557a
9f19f30
cc5d7aa
58a907f
05acbb6
28a6964
16942f1
145ace3
c42fc68
5846107
1486ad5
dff04ce
0ead039
4021498
9bca3f2
348b615
e0dcc39
210fd18
54d03f4
3462546
598170c
24a6f58
04a737d
9c4ddd7
f522d00
ecbd5f4
2729657
c75f6f0
457ca73
d33aea9
5c9b119
524dc94
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package logtest // import "go.opentelemetry.io/otel/log/logtest" | ||
|
|
||
| import ( | ||
| "context" | ||
| "time" | ||
|
|
||
| "github.com/google/go-cmp/cmp" | ||
| "github.com/google/go-cmp/cmp/cmpopts" | ||
|
|
||
| "go.opentelemetry.io/otel/log" | ||
| ) | ||
|
|
||
| // TestingT reports failure messages. | ||
| // [testing.T] implements this interface. | ||
| type TestingT interface { | ||
| Errorf(format string, args ...any) | ||
| } | ||
|
|
||
| // AssertEqual asserts that the two concrete data-types from the logtest package are equal. | ||
| func AssertEqual[T Recording | Record](t TestingT, want, got T, opts ...AssertOption) bool { | ||
| if h, ok := t.(interface{ Helper() }); ok { | ||
| h.Helper() | ||
| } | ||
|
|
||
| cmpOpts := []cmp.Option{ | ||
| cmp.Comparer(func(x, y context.Context) bool { return x == y }), // Compare context. | ||
| cmpopts.SortSlices(func(a, b log.KeyValue) bool { return a.Key < b.Key }), // Unordered compare of the key values. | ||
| cmpopts.EquateEmpty(), // Empty and nil collections are equal. | ||
| } | ||
|
|
||
| cfg := newAssertConfig(opts) | ||
| if cfg.ignoreTimestamp { | ||
| cmpOpts = append(cmpOpts, cmpopts.IgnoreTypes(time.Time{})) // Ignore Timestamps. | ||
| } | ||
|
|
||
| if diff := cmp.Diff(want, got, cmpOpts...); diff != "" { | ||
| t.Errorf("mismatch (-want +got):\n%s", diff) | ||
| return false | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| type assertConfig struct { | ||
| ignoreTimestamp bool | ||
| } | ||
|
|
||
| func newAssertConfig(opts []AssertOption) assertConfig { | ||
| var cfg assertConfig | ||
| for _, opt := range opts { | ||
| cfg = opt.apply(cfg) | ||
| } | ||
| return cfg | ||
| } | ||
|
|
||
| // AssertOption allows for fine grain control over how AssertEqual operates. | ||
| type AssertOption interface { | ||
| apply(cfg assertConfig) assertConfig | ||
| } | ||
|
|
||
| type fnOption func(cfg assertConfig) assertConfig | ||
|
|
||
| func (fn fnOption) apply(cfg assertConfig) assertConfig { | ||
| return fn(cfg) | ||
| } | ||
|
|
||
| // IgnoreTimestamp disables checking if timestamps are different. | ||
| func IgnoreTimestamp() AssertOption { | ||
| return fnOption(func(cfg assertConfig) assertConfig { | ||
| cfg.ignoreTimestamp = true | ||
| return cfg | ||
| }) | ||
| } | ||
|
Comment on lines
+69
to
+75
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SIG meeting: There was a proposal by @MrAlias to add some more generic function that would transform the logtest.AssertEqual(t, want, got, logtest.Mutate(func (r logtest.Record) logtest.Record {
r.Timestamp = time.Time{}
return r
}))Maybe https://pkg.go.dev/github.com/google/go-cmp/cmp#Transformer could used to achieve it. It would be better to add each option as a separate PR.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,174 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package logtest | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "go.opentelemetry.io/otel/log" | ||
| ) | ||
|
|
||
| var y2k = time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC) | ||
|
|
||
| type mockTestingT struct { | ||
| errors []string | ||
| } | ||
|
|
||
| func (m *mockTestingT) Errorf(format string, args ...any) { | ||
| m.errors = append(m.errors, format) | ||
| } | ||
|
|
||
| func TestAssertEqualRecording(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| a Recording | ||
| b Recording | ||
| opts []AssertOption | ||
| want bool | ||
| }{ | ||
| { | ||
| name: "equal recordings", | ||
| a: Recording{ | ||
| Scope{Name: t.Name()}: []Record{ | ||
| {Timestamp: y2k, Context: context.Background(), Attributes: []log.KeyValue{log.Int("n", 1), log.String("foo", "bar")}}, | ||
| }, | ||
| }, | ||
| b: Recording{ | ||
| Scope{Name: t.Name()}: []Record{ | ||
| {Timestamp: y2k, Context: context.Background(), Attributes: []log.KeyValue{log.String("foo", "bar"), log.Int("n", 1)}}, | ||
| }, | ||
| }, | ||
| want: true, | ||
| }, | ||
| { | ||
| name: "different recordings", | ||
| a: Recording{ | ||
| Scope{Name: t.Name()}: []Record{ | ||
| {Attributes: []log.KeyValue{log.String("foo", "bar")}}, | ||
| }, | ||
| }, | ||
| b: Recording{ | ||
| Scope{Name: t.Name()}: []Record{ | ||
| {Attributes: []log.KeyValue{log.Int("n", 1)}}, | ||
| }, | ||
| }, | ||
| want: false, | ||
| }, | ||
| { | ||
| name: "equal empty scopes", | ||
| a: Recording{ | ||
| Scope{Name: t.Name()}: nil, | ||
| }, | ||
| b: Recording{ | ||
| Scope{Name: t.Name()}: []Record{}, | ||
| }, | ||
| want: true, | ||
| }, | ||
| { | ||
| name: "equal empty attributes", | ||
| a: Recording{ | ||
| Scope{Name: t.Name()}: []Record{ | ||
| {Body: log.StringValue("msg"), Attributes: []log.KeyValue{}}, | ||
| }, | ||
| }, | ||
| b: Recording{ | ||
| Scope{Name: t.Name()}: []Record{ | ||
| {Body: log.StringValue("msg"), Attributes: nil}, | ||
| }, | ||
| }, | ||
| want: true, | ||
| }, | ||
| { | ||
| name: "ignore timestamp", | ||
| a: Recording{ | ||
| Scope{Name: t.Name()}: []Record{ | ||
| {Timestamp: y2k, Attributes: []log.KeyValue{log.String("foo", "bar")}}, | ||
| }, | ||
| }, | ||
| b: Recording{ | ||
| Scope{Name: t.Name()}: []Record{ | ||
| {Timestamp: time.Now(), Attributes: []log.KeyValue{log.String("foo", "bar")}}, | ||
| }, | ||
| }, | ||
| opts: []AssertOption{IgnoreTimestamp()}, | ||
| want: true, | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range tests { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| mockT := &mockTestingT{} | ||
| result := AssertEqual(mockT, tc.a, tc.b, tc.opts...) | ||
| if result != tc.want { | ||
| t.Errorf("AssertEqual() = %v, want %v", result, tc.want) | ||
| } | ||
| if !tc.want && len(mockT.errors) == 0 { | ||
| t.Errorf("expected Errorf call but got none") | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestAssertEqualRecord(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| a Record | ||
| b Record | ||
| opts []AssertOption | ||
| want bool | ||
| }{ | ||
| { | ||
| name: "equal records", | ||
| a: Record{ | ||
| Timestamp: y2k, | ||
| Context: context.Background(), | ||
| Attributes: []log.KeyValue{log.Int("n", 1), log.String("foo", "bar")}, | ||
| }, | ||
| b: Record{ | ||
| Timestamp: y2k, | ||
| Context: context.Background(), | ||
| Attributes: []log.KeyValue{log.String("foo", "bar"), log.Int("n", 1)}, | ||
| }, | ||
| want: true, | ||
| }, | ||
| { | ||
| name: "different records", | ||
| a: Record{ | ||
| Attributes: []log.KeyValue{log.String("foo", "bar")}, | ||
| }, | ||
| b: Record{ | ||
| Attributes: []log.KeyValue{log.Int("n", 1)}, | ||
| }, | ||
| want: false, | ||
| }, | ||
| { | ||
| name: "ignore timestamp", | ||
| a: Record{ | ||
| Timestamp: y2k, | ||
| Attributes: []log.KeyValue{log.String("foo", "bar")}, | ||
| }, | ||
| b: Record{ | ||
| Timestamp: time.Now(), | ||
| Attributes: []log.KeyValue{log.String("foo", "bar")}, | ||
| }, | ||
| opts: []AssertOption{IgnoreTimestamp()}, | ||
| want: true, | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range tests { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| mockT := &mockTestingT{} | ||
| result := AssertEqual(mockT, tc.a, tc.b, tc.opts...) | ||
| if result != tc.want { | ||
| t.Errorf("AssertEqual() = %v, want %v", result, tc.want) | ||
| } | ||
| if !tc.want && len(mockT.errors) == 0 { | ||
| t.Errorf("expected Errorf call but got none") | ||
| } | ||
| }) | ||
| } | ||
| } |
This file was deleted.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SIG meeting: There was a proposal by @MrAlias to add an option to add additional failure message.
It would be better to add each option as a separate PR.