Fixes test helper newChunk in the storage pkg. (#4056)

I wanted to test that overlapping chunks are correctly re-ordered
and bump into a rounding issue.
Keeping both for future will avoid the same mistake.

Signed-off-by: Cyril Tovena <[email protected]>
(cherry picked from commit 4c2feb3)

pkg/storage/store_test.go

@@ -1119,3 +1119,53 @@ func TestSchemaConfig_Validate(t *testing.T) {
 		})
 	}
 }
+
+func Test_OverlappingChunks(t *testing.T) {
+	chunks := []chunk.Chunk{
+
+		newChunk(logproto.Stream{
+			Labels: `{foo="bar"}`,
+			Entries: []logproto.Entry{
+				{Timestamp: time.Unix(0, 1), Line: "1"},
+				{Timestamp: time.Unix(0, 4), Line: "4"},
+			},
+		}),
+		newChunk(logproto.Stream{
+			Labels: `{foo="bar"}`,
+			Entries: []logproto.Entry{
+				{Timestamp: time.Unix(0, 2), Line: "2"},
+				{Timestamp: time.Unix(0, 3), Line: "3"},
+			},
+		}),
+	}
+	s := &store{
+		Store: &mockChunkStore{chunks: chunks, client: &mockChunkStoreClient{chunks: chunks}},
+		cfg: Config{
+			MaxChunkBatchSize: 10,
+		},
+		chunkMetrics: NilMetrics,
+	}
+
+	ctx = user.InjectOrgID(context.Background(), "test-user")
+	it, err := s.SelectLogs(ctx, logql.SelectLogParams{QueryRequest: &logproto.QueryRequest{
+		Selector:  `{foo="bar"}`,
+		Limit:     1000,
+		Direction: logproto.BACKWARD,
+		Start:     time.Unix(0, 0),
+		End:       time.Unix(0, 10),
+	}})
+	if err != nil {
+		t.Errorf("store.SelectLogs() error = %v", err)
+		return
+	}
+	defer it.Close()
+	require.True(t, it.Next())
+	require.Equal(t, "4", it.Entry().Line)
+	require.True(t, it.Next())
+	require.Equal(t, "3", it.Entry().Line)
+	require.True(t, it.Next())
+	require.Equal(t, "2", it.Entry().Line)
+	require.True(t, it.Next())
+	require.Equal(t, "1", it.Entry().Line)
+	require.False(t, it.Next())
+}

pkg/storage/util_test.go

@@ -20,10 +20,13 @@ import (
 	"github.com/grafana/loki/pkg/chunkenc"
 	"github.com/grafana/loki/pkg/logproto"
 	"github.com/grafana/loki/pkg/logql"
+	loki_util "github.com/grafana/loki/pkg/util"
 )
 
-var fooLabelsWithName = "{foo=\"bar\", __name__=\"logs\"}"
-var fooLabels = "{foo=\"bar\"}"
+var (
+	fooLabelsWithName = "{foo=\"bar\", __name__=\"logs\"}"
+	fooLabels         = "{foo=\"bar\"}"
+)
 
 var from = time.Unix(0, time.Millisecond.Nanoseconds())
 
@@ -96,15 +99,10 @@ func newChunk(stream logproto.Stream) chunk.Chunk {
 		builder.Set(labels.MetricName, "logs")
 		lbs = builder.Labels()
 	}
-	from, through := model.TimeFromUnixNano(stream.Entries[0].Timestamp.UnixNano()), model.TimeFromUnixNano(stream.Entries[0].Timestamp.UnixNano())
+
+	from, through := loki_util.RoundToMilliseconds(stream.Entries[0].Timestamp, stream.Entries[len(stream.Entries)-1].Timestamp)
 	chk := chunkenc.NewMemChunk(chunkenc.EncGZIP, 256*1024, 0)
 	for _, e := range stream.Entries {
-		if e.Timestamp.UnixNano() < from.UnixNano() {
-			from = model.TimeFromUnixNano(e.Timestamp.UnixNano())
-		}
-		if e.Timestamp.UnixNano() > through.UnixNano() {
-			through = model.TimeFromUnixNano(e.Timestamp.UnixNano())
-		}
 		_ = chk.Append(&e)
 	}
 	chk.Close()
@@ -154,8 +152,10 @@ type mockChunkStore struct {
 
 // mockChunkStore cannot implement both chunk.Store and chunk.Client,
 // since there is a conflict in signature for DeleteChunk method.
-var _ chunk.Store = &mockChunkStore{}
-var _ chunk.Client = &mockChunkStoreClient{}
+var (
+	_ chunk.Store  = &mockChunkStore{}
+	_ chunk.Client = &mockChunkStoreClient{}
+)
 
 func newMockChunkStore(streams []*logproto.Stream) *mockChunkStore {
 	chunks := make([]chunk.Chunk, 0, len(streams))
@@ -169,9 +169,11 @@ func (m *mockChunkStore) Put(ctx context.Context, chunks []chunk.Chunk) error {
 func (m *mockChunkStore) PutOne(ctx context.Context, from, through model.Time, chunk chunk.Chunk) error {
 	return nil
 }
+
 func (m *mockChunkStore) LabelValuesForMetricName(ctx context.Context, userID string, from, through model.Time, metricName string, labelName string) ([]string, error) {
 	return nil, nil
 }
+
 func (m *mockChunkStore) LabelNamesForMetricName(ctx context.Context, userID string, from, through model.Time, metricName string) ([]string, error) {
 	return nil, nil
 }
@@ -187,6 +189,7 @@ func (m *mockChunkStore) Stop() {}
 func (m *mockChunkStore) Get(ctx context.Context, userID string, from, through model.Time, matchers ...*labels.Matcher) ([]chunk.Chunk, error) {
 	return nil, nil
 }
+
 func (m *mockChunkStore) GetChunkFetcher(_ model.Time) *chunk.Fetcher {
 	return nil
 }