google / benchmark

A microbenchmark support library
Apache License 2.0
8.97k stars 1.62k forks source link

report_output_test regex fails to parse integer as float #301

Closed asuhl-formlabs closed 8 years ago

asuhl-formlabs commented 8 years ago

If cpu_time does not contain a decimal point the test fails:

benchmark/test/reporter_output_test.cc:46: Check: Check `remaining_output.eof() == false' failed. End of output reached before match for regex "^"BM_basic",[0-9]+,[0-9]+\.[0-9]+,[0-9]+\.[0-9]+,ns,,,,,$" was found
name,iterations,real_time,cpu_time,time_unit,bytes_per_second,items_per_second,label,error_occurred,error_message
"BM_basic",10000000,1.13567,1,ns,,,,,
"BM_error",,,,,,,,true,"message"
"BM_Complexity_O1/1",10000000,1.12328,1,ns,,,,,
"BM_Complexity_O1/8",14000000,1.12838,1.42857,ns,,,,,
"BM_Complexity_O1/64",10000000,1.12179,1,ns,,,,,
"BM_Complexity_O1/512",14000000,1.12677,1.42857,ns,,,,,
"BM_Complexity_O1/4k",14000000,1.12255,1.42857,ns,,,,,
"BM_Complexity_O1/32k",10000000,1.12003,1,ns,,,,,
"BM_Complexity_O1/256k",14000000,1.11966,1.42857,ns,,,,,
"BM_Complexity_O1_BigO",,1.12321,1.2449,(1),,,,,
"BM_Complexity_O1_RMS",,0.00270501,0.170366,,,,,,
dmah42 commented 8 years ago

From what i can see, this is fixed in the most recent code. At least, the regex being used for csv_report now uses a regex that doesn't assume a decimal point.

Are you on the latest?

asuhl-formlabs commented 8 years ago

I updated to latest and can't reproduce the problem --- but then again, my times aren't coming out to exact integers anymore, so it's hard to tell. There does still seem to be a regex that requires a decimal point on line 64 of test/reporter_output_test.cc: SET_SUBSTITUTIONS({{"%bigOStr", "[ ]*[0-9]+\\.[0-9]+ \\([0-9]+\\)"}, I understand this isn't the regex that was causing the error above, but it should probably be changed to not require a decimal point anyway.

EricWF commented 8 years ago

I understand this isn't the regex that was causing the error above, but it should probably be changed to not require a decimal point anyway.

I agree. Fixed in 2555035.

Closing this issue because I feel it has been addressed.