Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue of passing nil to formatters (from CSV PR) #252

Merged
merged 1 commit into from
Nov 3, 2018

Conversation

PragTob
Copy link
Member

@PragTob PragTob commented Nov 2, 2018

Interestingly the bug occurred thanks to our legacy format support.
Can't wait to get rid off it ;)

I changed the code for the formatter test to really make sure that
the same options passed to format/2 are also passed to write/2.
It didn't find the bug as the bug was in Configuration, but I
think it's still an improvement worth keeping.

Once this code is merged, we can delet the one unnecessary line
from the CSV formatter. Already tried it and it worked 馃帀

Interestingly the bug occurred thanks to our legacy format support.
Can't wait to get rid off it ;)

I changed the code for the formatter test to really make sure that
the same options passed to format/2 are also passed to write/2.
It didn't find the bug as the bug was in Configuration, but I
think it's still an improvement worth keeping.

Once this code is merged, we can delet the one unnecessary line
from the CSV formatter. Already tried it and it worked 馃帀
@PragTob PragTob force-pushed the trying-to-figure-out-nil-problem-in-benchee-csv branch from af6c178 to dddb98b Compare November 2, 2018 15:43
@PragTob
Copy link
Member Author

PragTob commented Nov 2, 2018

Falkyness:

  1) test .run_scenarios very fast function times are almost 0 with function call overhead elimination (Benchee.Benchmark.RunnerTest)
     test/benchee/benchmark/runner_test.exs:184
     Assertion with <= failed
     code:  assert median <= 60
     left:  183.0
     right: 60
     stacktrace:
       test/benchee/benchmark/runner_test.exs:196: (test)

Sad, restarted

@devonestes devonestes merged commit ee9f723 into master Nov 3, 2018
@devonestes devonestes deleted the trying-to-figure-out-nil-problem-in-benchee-csv branch November 3, 2018 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants