From 8ef3b925fcc0e31f70ed373c96ea83a0e9a0a389 Mon Sep 17 00:00:00 2001 From: asp24 <488588+asp24@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:38:14 +0000 Subject: [PATCH] Make the full test suite runnable `go test ./...` fails on master: TestNdjsonCountWhere2 dies with a nil pointer dereference inside loadFile. Two of the corpora fetched on demand answer 404 now, and the error path calls err.Error() on the nil error http.Get returns when the request itself succeeded and only the status was bad. loadFile now takes a testing.TB and skips the caller when a corpus cannot be obtained - there is nothing wrong with the code under test - while a corrupt archive or an unwritable cache stays fatal. It also closes the response body, which it never did. .gitignore was absent, so the corpora the tests download appear as untracked files; it now covers those, test binaries and profiling output. Test files and .gitignore only; no change to the library. --- .gitignore | 15 ++++++++++ ndjson_test.go | 60 +++++++++++++++++++++++++--------------- parse_json_amd64_test.go | 8 +++--- parsed_serialize_test.go | 6 ++-- 4 files changed, 59 insertions(+), 30 deletions(-) diff --git a/.gitignore b/.gitignore index e69de29..a9dc2f7 100644 --- a/.gitignore +++ b/.gitignore @@ -0,0 +1,15 @@ +/.idea/ +/.vscode/ + +# Corpora that loadFile fetches on demand rather than keeping in the repo. +/testdata/parking-citations-1M.json.zst +/testdata/RC_2009-01.json.zst + +# Build and profiling output. +*.test +*.out +*.prof + +# Editor leftovers. +*.swp +*.swo diff --git a/ndjson_test.go b/ndjson_test.go index 313378d..6a925a5 100644 --- a/ndjson_test.go +++ b/ndjson_test.go @@ -254,7 +254,7 @@ func TestNdjsonCountWhere(t *testing.T) { if testing.Short() { t.Skip("skipping... too long") } - ndjson := loadFile("testdata/parking-citations.json.zst") + ndjson := loadFile(t, "testdata/parking-citations.json.zst") pj, err := ParseND(ndjson, nil) if err != nil { t.Fatal(err) @@ -322,7 +322,7 @@ func TestNdjsonCountWhere2(t *testing.T) { if testing.Short() { t.Skip("skipping... too long") } - ndjson := loadFile("testdata/RC_2009-01.json.zst") + ndjson := loadFile(t, "testdata/RC_2009-01.json.zst") // Test trimming b := make([]byte, 0, len(ndjson)+4) b = append(b, '\n', '\n') @@ -363,11 +363,20 @@ func TestNdjsonCountWhere2(t *testing.T) { }) } -func loadFile(filename string) []byte { +// loadFile returns a test corpus, fetching and caching it when the repository does +// not ship the file. +// +// The corpora too large to commit live on an external host, and it no longer serves +// all of them - parking-citations-1M.json.zst and RC_2009-01.json.zst both answer +// 404. A corpus that cannot be obtained skips the caller rather than failing it: +// there is nothing wrong with the code under test, and a hard failure there made +// `go test ./...` unusable. Anything else is still fatal. +func loadFile(tb testing.TB, filename string) []byte { + tb.Helper() if !strings.HasSuffix(filename, ".zst") { ndjson, err := ioutil.ReadFile(filename) if err != nil { - panic("Failed to load file") + tb.Skipf("corpus %s is unavailable: %v", filename, err) } return ndjson } @@ -379,31 +388,36 @@ func loadFile(filename string) []byte { defer f.Close() break } - if os.IsNotExist(err) { - fmt.Println("downloading file", filename) - resp, err := http.DefaultClient.Get("https://dl.minio.io/assets/" + filepath.Base(filename)) - if err == nil && resp.StatusCode == http.StatusOK { - b, err := ioutil.ReadAll(resp.Body) - if err == nil { - err = ioutil.WriteFile(filename, b, os.ModePerm) - if err == nil { - continue - } - panic("Failed to write file:" + err.Error()) - } - panic("Failed to read file:" + err.Error()) - } - panic("Failed to download file:" + err.Error()) + if !os.IsNotExist(err) { + tb.Fatalf("opening %s: %v", filename, err) + } + fmt.Println("downloading file", filename) + resp, err := http.DefaultClient.Get("https://dl.minio.io/assets/" + filepath.Base(filename)) + if err != nil { + tb.Skipf("corpus %s could not be downloaded: %v", filename, err) + } + // err is nil here, so it cannot be used to describe a bad status. + if resp.StatusCode != http.StatusOK { + resp.Body.Close() + tb.Skipf("corpus %s could not be downloaded: %s", filename, resp.Status) + } + b, err := ioutil.ReadAll(resp.Body) + resp.Body.Close() + if err != nil { + tb.Skipf("corpus %s could not be read: %v", filename, err) + } + if err = ioutil.WriteFile(filename, b, os.ModePerm); err != nil { + tb.Fatalf("writing %s: %v", filename, err) } } dec, err := zstd.NewReader(f) if err != nil { - panic("Failed to create decompressor") + tb.Fatalf("creating decompressor for %s: %v", filename, err) } defer dec.Close() ndjson, err := ioutil.ReadAll(dec) if err != nil { - panic("Failed to load file") + tb.Fatalf("decompressing %s: %v", filename, err) } return ndjson } @@ -490,7 +504,7 @@ func BenchmarkNdjsonWarmCountStar(b *testing.B) { b.SkipNow() } - ndjson := loadFile("testdata/parking-citations-1M.json.zst") + ndjson := loadFile(b, "testdata/parking-citations-1M.json.zst") pj, err := ParseND(ndjson, nil) if err != nil { @@ -510,7 +524,7 @@ func BenchmarkNdjsonWarmCountStarWithWhere(b *testing.B) { b.SkipNow() } - ndjson := loadFile("testdata/parking-citations-1M.json.zst") + ndjson := loadFile(b, "testdata/parking-citations-1M.json.zst") pj, err := ParseND(ndjson, nil) if err != nil { diff --git a/parse_json_amd64_test.go b/parse_json_amd64_test.go index 3a3e26a..463a154 100644 --- a/parse_json_amd64_test.go +++ b/parse_json_amd64_test.go @@ -78,7 +78,7 @@ func BenchmarkNdjsonStage2(b *testing.B) { b.SkipNow() } - ndjson := loadFile("testdata/parking-citations-1M.json.zst") + ndjson := loadFile(b, "testdata/parking-citations-1M.json.zst") pj := internalParsedJson{} b.SetBytes(int64(len(ndjson))) @@ -96,7 +96,7 @@ func BenchmarkNdjsonStage1(b *testing.B) { if !SupportedCPU() { b.SkipNow() } - ndjson := loadFile("testdata/parking-citations-1M.json.zst") + ndjson := loadFile(b, "testdata/parking-citations-1M.json.zst") pj := internalParsedJson{} @@ -116,7 +116,7 @@ func BenchmarkNdjsonColdCountStar(b *testing.B) { if !SupportedCPU() { b.SkipNow() } - ndjson := loadFile("testdata/parking-citations-1M.json.zst") + ndjson := loadFile(b, "testdata/parking-citations-1M.json.zst") b.SetBytes(int64(len(ndjson))) b.ReportAllocs() @@ -135,7 +135,7 @@ func BenchmarkNdjsonColdCountStarWithWhere(b *testing.B) { if !SupportedCPU() { b.SkipNow() } - ndjson := loadFile("testdata/parking-citations-1M.json.zst") + ndjson := loadFile(b, "testdata/parking-citations-1M.json.zst") const want = 110349 runtime.GC() pj := internalParsedJson{} diff --git a/parsed_serialize_test.go b/parsed_serialize_test.go index d8777fd..fcb8f3b 100644 --- a/parsed_serialize_test.go +++ b/parsed_serialize_test.go @@ -129,7 +129,7 @@ func BenchmarkSerializeNDJSON(b *testing.B) { b.SkipNow() } - ndjson := loadFile("testdata/parking-citations-1M.json.zst") + ndjson := loadFile(b, "testdata/parking-citations-1M.json.zst") pj, err := ParseND(ndjson, nil) if err != nil { @@ -172,7 +172,7 @@ func BenchmarkDeSerializeNDJSON(b *testing.B) { b.SkipNow() } - ndjson := loadFile("testdata/parking-citations-1M.json.zst") + ndjson := loadFile(b, "testdata/parking-citations-1M.json.zst") pj, err := ParseND(ndjson, nil) if err != nil { @@ -224,7 +224,7 @@ func TestDeSerializeNDJSON(t *testing.T) { if testing.Short() { t.Skip("skipping... too long") } - ndjson := loadFile("testdata/parking-citations.json.zst") + ndjson := loadFile(t, "testdata/parking-citations.json.zst") pj, err := ParseND(ndjson, nil) if err != nil {