app: consistently use t.Fatal* instead of t.Error* (except of app/vmalert and app/vmctl - these packages will be processed in a separate commit)

Consistently using t.Fatal* simplifies the test code and makes it less fragile, since it is common error
to forget to make proper cleanup after t.Error* call. Also t.Error* calls do not provide any practical
benefits when some tests fail. They just clutter test output with additional noise information,
which do not help in fixing failing tests most of the time.

While at it, improve errors generated at app/victoria-metrics tests, so they contain more useful information
when debugging failed tests.

This is a follow-up for a9525da8a4
This commit is contained in:
Aliaksandr Valialkin 2024-07-11 15:53:40 +02:00
parent fabf0b928e
commit 62dabd67a2
No known key found for this signature in database
GPG key ID: 52C003EE2BCDB9EB
7 changed files with 111 additions and 90 deletions

View file

@ -3,6 +3,7 @@ package main
import (
"bytes"
"encoding/json"
"errors"
"flag"
"fmt"
"io"
@ -249,24 +250,24 @@ func TestWriteRead(t *testing.T) {
func testWrite(t *testing.T) {
t.Run("prometheus", func(t *testing.T) {
for _, test := range readIn("prometheus", t, insertionTime) {
for _, test := range readIn("prometheus", insertionTime) {
if test.Data == nil {
continue
}
s := newSuite(t)
r := testutil.WriteRequest{}
s.noError(json.Unmarshal([]byte(strings.Join(test.Data, "\n")), &r.Timeseries))
data, err := testutil.Compress(r)
s.greaterThan(len(r.Timeseries), 0)
if err != nil {
t.Errorf("error compressing %v %s", r, err)
t.Fail()
testData := strings.Join(test.Data, "\n")
if err := json.Unmarshal([]byte(testData), &r.Timeseries); err != nil {
panic(fmt.Errorf("BUG: cannot unmarshal TimeSeries: %s\ntest data\n%s", err, testData))
}
if n := len(r.Timeseries); n <= 0 {
panic(fmt.Errorf("BUG: expecting non-empty Timeseries in test data:\n%s", testData))
}
data := testutil.Compress(r)
httpWrite(t, testPromWriteHTTPPath, test.InsertQuery, bytes.NewBuffer(data))
}
})
t.Run("csv", func(t *testing.T) {
for _, test := range readIn("csv", t, insertionTime) {
for _, test := range readIn("csv", insertionTime) {
if test.Data == nil {
continue
}
@ -275,7 +276,7 @@ func testWrite(t *testing.T) {
})
t.Run("influxdb", func(t *testing.T) {
for _, x := range readIn("influxdb", t, insertionTime) {
for _, x := range readIn("influxdb", insertionTime) {
test := x
t.Run(test.Name, func(t *testing.T) {
t.Parallel()
@ -284,7 +285,7 @@ func testWrite(t *testing.T) {
}
})
t.Run("graphite", func(t *testing.T) {
for _, x := range readIn("graphite", t, insertionTime) {
for _, x := range readIn("graphite", insertionTime) {
test := x
t.Run(test.Name, func(t *testing.T) {
t.Parallel()
@ -293,7 +294,7 @@ func testWrite(t *testing.T) {
}
})
t.Run("opentsdb", func(t *testing.T) {
for _, x := range readIn("opentsdb", t, insertionTime) {
for _, x := range readIn("opentsdb", insertionTime) {
test := x
t.Run(test.Name, func(t *testing.T) {
t.Parallel()
@ -302,7 +303,7 @@ func testWrite(t *testing.T) {
}
})
t.Run("opentsdbhttp", func(t *testing.T) {
for _, x := range readIn("opentsdbhttp", t, insertionTime) {
for _, x := range readIn("opentsdbhttp", insertionTime) {
test := x
t.Run(test.Name, func(t *testing.T) {
t.Parallel()
@ -316,7 +317,7 @@ func testWrite(t *testing.T) {
func testRead(t *testing.T) {
for _, engine := range []string{"csv", "prometheus", "graphite", "opentsdb", "influxdb", "opentsdbhttp"} {
t.Run(engine, func(t *testing.T) {
for _, x := range readIn(engine, t, insertionTime) {
for _, x := range readIn(engine, insertionTime) {
test := x
t.Run(test.Name, func(t *testing.T) {
t.Parallel()
@ -365,11 +366,10 @@ func testRead(t *testing.T) {
}
}
func readIn(readFor string, t *testing.T, insertTime time.Time) []test {
t.Helper()
s := newSuite(t)
func readIn(readFor string, insertTime time.Time) []test {
testDir := filepath.Join(testFixturesDir, readFor)
var tt []test
s.noError(filepath.Walk(filepath.Join(testFixturesDir, readFor), func(path string, _ os.FileInfo, err error) error {
err := filepath.Walk(testDir, func(path string, _ os.FileInfo, err error) error {
if err != nil {
return err
}
@ -377,84 +377,129 @@ func readIn(readFor string, t *testing.T, insertTime time.Time) []test {
return nil
}
b, err := os.ReadFile(path)
s.noError(err)
if err != nil {
panic(fmt.Errorf("BUG: cannot read %s: %s", path, err))
}
item := test{}
s.noError(json.Unmarshal(b, &item))
if err := json.Unmarshal(b, &item); err != nil {
panic(fmt.Errorf("cannot parse %T from %s: %s; data:\n%s", &item, path, err, b))
}
for i := range item.Data {
item.Data[i] = testutil.PopulateTimeTplString(item.Data[i], insertTime)
}
tt = append(tt, item)
return nil
}))
})
if err != nil {
panic(fmt.Errorf("BUG: cannot read test data at %s: %w", testDir, err))
}
if len(tt) == 0 {
t.Fatalf("no test found in %s", filepath.Join(testFixturesDir, readFor))
panic(fmt.Errorf("BUG: no tests found in %s", testDir))
}
return tt
}
func httpWrite(t *testing.T, address, query string, r io.Reader) {
t.Helper()
s := newSuite(t)
resp, err := http.Post(address+query, "", r)
s.noError(err)
s.noError(resp.Body.Close())
s.equalInt(resp.StatusCode, 204)
requestURL := address + query
resp, err := http.Post(requestURL, "", r)
if err != nil {
t.Fatalf("cannot send request to %s: %s", requestURL, err)
}
_ = resp.Body.Close()
if resp.StatusCode != 204 {
t.Fatalf("unexpected status code received from %s; got %d; want 204", requestURL, resp.StatusCode)
}
}
func tcpWrite(t *testing.T, address string, data string) {
func tcpWrite(t *testing.T, address, data string) {
t.Helper()
s := newSuite(t)
conn, err := net.Dial("tcp", address)
s.noError(err)
if err != nil {
t.Fatalf("cannot dial %s: %s", address, err)
}
defer func() {
_ = conn.Close()
}()
n, err := conn.Write([]byte(data))
s.noError(err)
s.equalInt(n, len(data))
if err != nil {
t.Fatalf("cannot write %d bytes to %s: %s", len(data), address, err)
}
if n != len(data) {
panic(fmt.Errorf("BUG: conn.Write() returned unexpected number of written bytes to %s; got %d; want %d", address, n, len(data)))
}
}
func httpReadMetrics(t *testing.T, address, query string) []Metric {
t.Helper()
s := newSuite(t)
resp, err := http.Get(address + query)
s.noError(err)
requestURL := address + query
resp, err := http.Get(requestURL)
if err != nil {
t.Fatalf("cannot send request to %s: %s", requestURL, err)
}
defer func() {
_ = resp.Body.Close()
}()
s.equalInt(resp.StatusCode, 200)
if resp.StatusCode != 200 {
t.Fatalf("unexpected status code received from %s; got %d; want 200", requestURL, resp.StatusCode)
}
var rows []Metric
for dec := json.NewDecoder(resp.Body); dec.More(); {
dec := json.NewDecoder(resp.Body)
for {
var row Metric
s.noError(dec.Decode(&row))
err := dec.Decode(&row)
if err != nil {
if errors.Is(err, io.EOF) {
return rows
}
t.Fatalf("cannot decode %T from response received from %s: %s", &row, requestURL, err)
}
rows = append(rows, row)
}
return rows
}
func httpReadStruct(t *testing.T, address, query string, dst any) {
t.Helper()
s := newSuite(t)
resp, err := http.Get(address + query)
s.noError(err)
requestURL := address + query
resp, err := http.Get(requestURL)
if err != nil {
t.Fatalf("cannot send request to %s: %s", requestURL, err)
}
defer func() {
_ = resp.Body.Close()
}()
s.equalInt(resp.StatusCode, 200)
s.noError(json.NewDecoder(resp.Body).Decode(dst))
if resp.StatusCode != 200 {
t.Fatalf("unexpected status code received from %s; got %d; want 200", requestURL, resp.StatusCode)
}
err = json.NewDecoder(resp.Body).Decode(dst)
if err != nil {
t.Fatalf("cannot decode %T from response received from %s: %s", dst, requestURL, err)
}
}
func httpReadData(t *testing.T, address, query string) []byte {
t.Helper()
s := newSuite(t)
resp, err := http.Get(address + query)
s.noError(err)
requestURL := address + query
resp, err := http.Get(requestURL)
if err != nil {
t.Fatalf("cannot send request to %s: %s", requestURL, err)
}
defer func() {
_ = resp.Body.Close()
}()
s.equalInt(resp.StatusCode, 200)
if resp.StatusCode != 200 {
t.Fatalf("unexpected status code received from %s; got %d; want 200", requestURL, resp.StatusCode)
}
data, err := io.ReadAll(resp.Body)
s.noError(err)
if err != nil {
t.Fatalf("cannot read response from %s: %s", requestURL, err)
}
return data
}
@ -503,34 +548,6 @@ func removeIfFoundSeries(r map[string]string, contains []map[string]string) []ma
return contains
}
type suite struct{ t *testing.T }
func newSuite(t *testing.T) *suite { return &suite{t: t} }
func (s *suite) noError(err error) {
s.t.Helper()
if err != nil {
s.t.Errorf("unexpected error %v", err)
s.t.FailNow()
}
}
func (s *suite) equalInt(a, b int) {
s.t.Helper()
if a != b {
s.t.Errorf("%d not equal %d", a, b)
s.t.FailNow()
}
}
func (s *suite) greaterThan(a, b int) {
s.t.Helper()
if a <= b {
s.t.Errorf("%d less or equal then %d", a, b)
s.t.FailNow()
}
}
func TestImportJSONLines(t *testing.T) {
f := func(labelsCount, labelLen int) {
t.Helper()

View file

@ -1,12 +1,16 @@
package test
import "github.com/golang/snappy"
import (
"fmt"
"github.com/golang/snappy"
)
// Compress marshals and compresses wr.
func Compress(wr WriteRequest) ([]byte, error) {
func Compress(wr WriteRequest) []byte {
data, err := wr.Marshal()
if err != nil {
return nil, err
panic(fmt.Errorf("BUG: cannot compress WriteRequest: %s", err))
}
return snappy.Encode(nil, data), nil
return snappy.Encode(nil, data)
}

View file

@ -26,11 +26,11 @@ func TestInsertHandler(t *testing.T) {
req := httptest.NewRequest(http.MethodPost, "/insert/0/api/v1/import/prometheus", bytes.NewBufferString(`{"foo":"bar"}
go_memstats_alloc_bytes_total 1`))
if err := InsertHandler(nil, req); err != nil {
t.Errorf("unxepected error %s", err)
t.Fatalf("unxepected error %s", err)
}
expectedMsg := "cannot unmarshal Prometheus line"
if !strings.Contains(testOutput.String(), expectedMsg) {
t.Errorf("output %q should contain %q", testOutput.String(), expectedMsg)
t.Fatalf("output %q should contain %q", testOutput.String(), expectedMsg)
}
}

View file

@ -74,10 +74,7 @@ func writeInputSeries(input []series, interval *promutils.Duration, startStamp t
r.Timeseries = append(r.Timeseries, testutil.TimeSeries{Labels: ls, Samples: samples})
}
data, err := testutil.Compress(r)
if err != nil {
return fmt.Errorf("failed to compress data: %v", err)
}
data := testutil.Compress(r)
// write input series to vm
httpWrite(dst, bytes.NewBuffer(data))
vmstorage.Storage.DebugFlush()

View file

@ -8,15 +8,18 @@ import (
func TestHasFilepathPrefix(t *testing.T) {
f := func(dst, storageDataPath string, resultExpected bool) {
t.Helper()
result := hasFilepathPrefix(dst, storageDataPath)
if result != resultExpected {
t.Errorf("unexpected hasFilepathPrefix(%q, %q); got: %v; want: %v", dst, storageDataPath, result, resultExpected)
t.Fatalf("unexpected hasFilepathPrefix(%q, %q); got: %v; want: %v", dst, storageDataPath, result, resultExpected)
}
}
pwd, err := filepath.Abs("")
if err != nil {
t.Fatalf("cannot determine working directory: %s", err)
}
f("s3://foo/bar", "foo", false)
f("fs://"+pwd+"/foo", "foo", true)
f("fs://"+pwd+"/foo", "foo/bar", false)

View file

@ -103,7 +103,7 @@ func TestGetSumInstantValues(t *testing.T) {
result := getSumInstantValues(nil, cached, start, end, timestamp)
if !reflect.DeepEqual(result, expectedResult) {
t.Errorf("unexpected result; got\n%v\nwant\n%v", result, expectedResult)
t.Fatalf("unexpected result; got\n%v\nwant\n%v", result, expectedResult)
}
}
ts := func(name string, timestamp int64, value float64) *timeseries {

View file

@ -46,7 +46,7 @@ func TestVmrangeBucketsToLE(t *testing.T) {
result := vmrangeBucketsToLE(tss)
resultBuckets := timeseriesToPromMetrics(result)
if !reflect.DeepEqual(resultBuckets, bucketsExpected) {
t.Errorf("unexpected vmrangeBucketsToLE(); got\n%v\nwant\n%v", resultBuckets, bucketsExpected)
t.Fatalf("unexpected vmrangeBucketsToLE(); got\n%v\nwant\n%v", resultBuckets, bucketsExpected)
}
}