fix nil allocationSet edge case

pkg/kubecost/allocation.go

@@ -2287,6 +2287,12 @@ func (asr *AllocationSetRange) AccumulateBy(resolution time.Duration) (*Allocati
 		return allocSetRange, nil
 	}
 
+	// two ways to get end of asr window
+	// 1. check if last set of asr
+	// 2. check if set window.end is asr.window.end
+	// both require no lock to be present
+	// option 1: 2 asr.window methods
+	// option 2: 2 window.length methods
 	asrWindow := asr.window()
 	asrResolution := asrWindow.Duration()
 
@@ -2305,21 +2311,16 @@ func (asr *AllocationSetRange) AccumulateBy(resolution time.Duration) (*Allocati
 		if err != nil {
 			return nil, err
 		}
-		currAccumulatedSum += allocSet.Window.Duration()
-
-		// fmt.Printf("asr end %v as end %v\n", asrWindow.end, allocSet.Window.end)
-		// two ways to get end of asr window
-		// 1. check if last set of asr
-		// 2. check if set window.end is asr.window.end
-		// both require no lock to be present
-		// option 1: 2 asr.window methods
-		// option 2: 2 window.length methods
-
-		// check if end of asr to sum the final set too
-		if currAccumulatedSum >= resolution || NewWindow(nil, asrWindow.end).Equal(NewWindow(nil, allocSet.Window.end)) {
-			allocSetRange.allocations = append(allocSetRange.allocations, allocSet)
-			allocSet = &AllocationSet{allocations: map[string]*Allocation{}}
-			currAccumulatedSum = 0
+
+		if allocSet != nil {
+			currAccumulatedSum += allocSet.Window.Duration()
+
+			// check if end of asr to sum the final set too
+			if currAccumulatedSum >= resolution || NewWindow(nil, asrWindow.end).Equal(NewWindow(nil, allocSet.Window.end)) {
+				allocSetRange.allocations = append(allocSetRange.allocations, allocSet)
+				allocSet = &AllocationSet{allocations: map[string]*Allocation{}}
+				currAccumulatedSum = 0
+			}
 		}
 	}
 

pkg/kubecost/allocation_test.go

@@ -2204,43 +2204,50 @@ func TestAllocationSetRange_AccumulateBy(t *testing.T) {
 		t.Fatalf("accumulating AllocationSetRange: expected first allocationSet window duration to be %v; actual %v", dur, result.allocations[1].Resolution())
 	}
 
-	// // Accumulate non-nil with nil should result in copy of non-nil, regardless of order
-	// accumulateBy -> accumulate -> returns todayAs to set as result but nil pointer dereference error?
-	// result, err = NewAllocationSetRange(nil, todayAS).AccumulateBy(dur)
-	// if err != nil {
-	// 	t.Fatalf("unexpected error accumulating AllocationSetRange of length 1: %s", err)
-	// }
-	// if result == nil {
-	// 	t.Fatalf("accumulating AllocationSetRange: expected AllocationSet; actual %s", result)
-	// }
-
-	// for _, as := range result.allocations {
-	// 	if as.TotalCost() != 6.0 {
-	// 		t.Fatalf("accumulating AllocationSetRange: expected total cost 6.0; actual %f", as.TotalCost())
-	// 	}
-	// }
-
-	// result, err = NewAllocationSetRange(todayAS, nil).Accumulate()
-	// if err != nil {
-	// 	t.Fatalf("unexpected error accumulating AllocationSetRange of length 1: %s", err)
-	// }
-	// if result == nil {
-	// 	t.Fatalf("accumulating AllocationSetRange: expected AllocationSet; actual %s", result)
-	// }
-	// if result.TotalCost() != 6.0 {
-	// 	t.Fatalf("accumulating AllocationSetRange: expected total cost 6.0; actual %f", result.TotalCost())
-	// }
-
-	// result, err = NewAllocationSetRange(nil, todayAS, nil).Accumulate()
-	// if err != nil {
-	// 	t.Fatalf("unexpected error accumulating AllocationSetRange of length 1: %s", err)
-	// }
-	// if result == nil {
-	// 	t.Fatalf("accumulating AllocationSetRange: expected AllocationSet; actual %s", result)
-	// }
-	// if result.TotalCost() != 6.0 {
-	// 	t.Fatalf("accumulating AllocationSetRange: expected total cost 6.0; actual %f", result.TotalCost())
-	// }
+	// Accumulate non-nil with nil should result in copy of non-nil, regardless of order
+	result, err = NewAllocationSetRange(nil, todayAS).AccumulateBy(dur)
+	if err != nil {
+		t.Fatalf("unexpected error accumulating AllocationSetRange of length 1: %s", err)
+	}
+	if result == nil {
+		t.Fatalf("accumulating AllocationSetRange: expected AllocationSet; actual %s", result)
+	}
+
+	for _, as := range result.allocations {
+		if as.TotalCost() != 6.0 {
+			t.Fatalf("accumulating AllocationSetRange: expected total cost 6.0; actual %f", as.TotalCost())
+		}
+	}
+
+	result, err = NewAllocationSetRange(todayAS, nil).AccumulateBy(dur)
+	if err != nil {
+		t.Fatalf("unexpected error accumulating AllocationSetRange of length 1: %s", err)
+	}
+	if result == nil {
+		t.Fatalf("accumulating AllocationSetRange: expected AllocationSet; actual %s", result)
+	}
+	sumCost = 0.0
+	for _, as := range result.allocations {
+		sumCost += as.TotalCost()
+	}
+	if sumCost != 6.0 {
+		t.Fatalf("accumulating AllocationSetRange: expected total cost 6.0; actual %f", sumCost)
+	}
+
+	result, err = NewAllocationSetRange(nil, todayAS, nil).AccumulateBy(dur)
+	if err != nil {
+		t.Fatalf("unexpected error accumulating AllocationSetRange of length 1: %s", err)
+	}
+	if result == nil {
+		t.Fatalf("accumulating AllocationSetRange: expected AllocationSet; actual %s", result)
+	}
+	sumCost = 0.0
+	for _, as := range result.allocations {
+		sumCost += as.TotalCost()
+	}
+	if sumCost != 6.0 {
+		t.Fatalf("accumulating AllocationSetRange: expected total cost 6.0; actual %f", sumCost)
+	}
 
 	// // Accumulate three non-nil should result in sum of both with appropriate start, end
 	result, err = NewAllocationSetRange(ago2dAS, yesterdayAS, todayAS).AccumulateBy(dur)
@@ -2300,9 +2307,9 @@ func TestAllocationSetRange_AccumulateBy(t *testing.T) {
 	if alloc.RAMEfficiency() != 1.0 {
 		t.Fatalf("accumulating AllocationSetRange: expected 1.0; actual %f", alloc.RAMEfficiency())
 	}
-	// if alloc.TotalCost() != 12.0 {
-	// 	t.Fatalf("accumulating AllocationSetRange: expected 12.0; actual %f", alloc.TotalCost())
-	// }
+	if alloc.TotalCost() != 12.0 {
+		t.Fatalf("accumulating AllocationSetRange: expected 12.0; actual %f", alloc.TotalCost())
+	}
 	if alloc.TotalEfficiency() != 1.0 {
 		t.Fatalf("accumulating AllocationSetRange: expected 1.0; actual %f", alloc.TotalEfficiency())
 	}