appview/indexer: add failing tests for Search nil,nil error swallowing #1288

Summary#

Documents a bug where Search in all three indexers (repos, issues, pulls) returns nil, nil when the underlying bleve SearchInContext call fails. Every caller does if err != nil { return } — the guard is bypassed, and the next dereference of the nil result panics.

This PR adds TestSearchOnClosedIndex to each package to reproduce the bug. The tests intentionally fail to prove the bug is present. The fix (return nil, nil → return nil, err) comes in a follow-up PR.

Test plan#

go test ./appview/indexer/repos/ -run TestSearchOnClosedIndex → FAIL
go test ./appview/indexer/issues/ -run TestSearchOnClosedIndex → FAIL
go test ./appview/indexer/pulls/ -run TestSearchOnClosedIndex → FAIL
All pre-existing tests in those three packages still PASS

+71

3 changed files

Diff round #0

expand all

appview

indexer

issues

indexer_test.go

pulls

indexer_test.go

repos

indexer_test.go

+12

appview/indexer/issues/indexer_test.go

··· 342 342 assert.Equal(t, uint64(1), result.Total) 343 343 assert.Contains(t, result.Hits, int64(1)) 344 344 } 345 + 346 + func TestSearchOnClosedIndex(t *testing.T) { 347 + ix, cleanup := setupTestIndexer(t) 348 + // close the index before searching to force an error from bleve 349 + cleanup() 350 + 351 + _, err := ix.Search(context.Background(), models.IssueSearchOptions{ 352 + Keywords: []string{"anything"}, 353 + Page: pagination.Page{Limit: 10}, 354 + }) 355 + require.Error(t, err, "Search on a closed index must return a non-nil error, not nil,nil") 356 + }

+47

appview/indexer/pulls/indexer_test.go

··· 1 + package pulls_indexer 2 + 3 + import ( 4 + "context" 5 + "os" 6 + "testing" 7 + 8 + "github.com/blevesearch/bleve/v2" 9 + "github.com/stretchr/testify/require" 10 + "tangled.org/core/appview/models" 11 + "tangled.org/core/appview/pagination" 12 + ) 13 + 14 + func setupTestIndexer(t *testing.T) (*Indexer, func()) { 15 + t.Helper() 16 + 17 + tmpDir, err := os.MkdirTemp("", "pull_indexer_test") 18 + require.NoError(t, err) 19 + 20 + ix := NewIndexer(tmpDir) 21 + 22 + mapping, err := generatePullIndexMapping() 23 + require.NoError(t, err) 24 + 25 + indexer, err := bleve.New(tmpDir, mapping) 26 + require.NoError(t, err) 27 + ix.indexer = indexer 28 + 29 + cleanup := func() { 30 + ix.indexer.Close() 31 + os.RemoveAll(tmpDir) 32 + } 33 + 34 + return ix, cleanup 35 + } 36 + 37 + func TestSearchOnClosedIndex(t *testing.T) { 38 + ix, cleanup := setupTestIndexer(t) 39 + // close the index before searching to force an error from bleve 40 + cleanup() 41 + 42 + _, err := ix.Search(context.Background(), models.PullSearchOptions{ 43 + Keywords: []string{"anything"}, 44 + Page: pagination.Page{Limit: 10}, 45 + }) 46 + require.Error(t, err, "Search on a closed index must return a non-nil error, not nil,nil") 47 + }

+12

appview/indexer/repos/indexer_test.go

··· 596 596 assert.Equal(t, uint64(2), result.Total) 597 597 } 598 598 599 + func TestSearchOnClosedIndex(t *testing.T) { 600 + ix, cleanup := setupTestIndexer(t) 601 + // close the index before searching to force an error from bleve 602 + cleanup() 603 + 604 + _, err := ix.Search(context.Background(), models.RepoSearchOptions{ 605 + Keywords: []string{"anything"}, 606 + Page: pagination.Page{Limit: 10}, 607 + }) 608 + require.Error(t, err, "Search on a closed index must return a non-nil error, not nil,nil") 609 + } 610 + 599 611 func TestDelete(t *testing.T) { 600 612 ix, cleanup := setupTestIndexer(t) 601 613 defer cleanup()

History

1 round 1 comment

matias.tngl.sh submitted #0 8d

1 commit

expand

47cc71b4

appview/indexer: add failing tests for Search nil,nil bug

no conflicts, ready to merge

expand 1 comment

oppi.li 6d

do we need these tests yet?

appview/indexer/repos/indexer_test.go:608: this error message would now be outdated by the merge of the follow-up fix, would it not?