bookingcom/carbonapi

Faulty floats comparison leads to tests intermittently failing on some machines. Rewrite floats comparison.

grzkv opened this issue · 0 comments

grzkv commented

The comparison

if !reflect.DeepEqual(r[0].Values, gg.Values) || !reflect.DeepEqual(r[0].IsAbsent, gg.IsAbsent) ||

is comparing floats byte-by-byte without taking into account the limited precision of floating-point representation. This causes the test to fail on my machine while being successful in CI and most other machines.

The comparison fails between the following structs

&types.MetricData{Metric:types.Metric{Name:"stddevSeries(metric1,metric2,metric3)", StartTime:1654694091, StopTime:1654694096, StepTime:1, Values:[]float64{0.4714045207910317, 0.9428090415820634, 1.4142135623730951, 1.8856180831641267, 2.3570226039551585}, IsAbsent:[]bool{false, false, false, false, false}}, GraphOptions:types.GraphOptions{}, ValuesPerPoint:0, AggregateFunction:(func([]float64, []bool) (float64, bool))(nil)},
&types.MetricData{Metric:types.Metric{Name:"stddevSeries(metric1,metric2,metric3)", StartTime:1654694091, StopTime:1654694096, StepTime:1, Values:[]float64{0.4714045207910317, 0.9428090415820634, 1.4142135623730951, 1.8856180831641267, 2.357022603955158}, IsAbsent:[]bool{false, false, false, false, false}}, GraphOptions:types.GraphOptions{}, ValuesPerPoint:0, AggregateFunction:(func([]float64, []bool) (float64, bool))(nil)}}

The only difference is between the last elements of Values: 2.3570226039551585 vs 2.357022603955158. These values are the same within the precision limits. Our code incorrectly treats them as being different.

We need to re-write the floats comparison in the failing test. We can limit this issue to re-writing TestMultiReturnEvalExpr for now. This will already cover multiple cases. There already are attempts to do that in the code:

func NearlyEqualMetrics(a, b *types.MetricData) bool {

So, this is likely not a completely new problem.