gavinhoward / bc

An implementation of the POSIX bc calculator with GNU extensions and dc, moved away from GitHub. Finished, but well-maintained.

https://git.gavinhoward.com/gavin/bc

Other

162 stars 32 forks source link

Add a test to compare negative numbers #25

Closed eg15 closed 4 years ago

eg15 commented 4 years ago

Looks like I found a bug. "-1000000000 < -1" returns 0 (false) on my machine which is obviously wrong. The fix will follow.

codecov[bot] commented 4 years ago

Codecov Report

Merging #25 into master will not change coverage. The diff coverage is 100%.

@@           Coverage Diff           @@
##           master      #25   +/-   ##
=======================================
  Coverage   99.71%   99.71%           
=======================================
  Files          16       16           
  Lines        3478     3478           
=======================================
  Hits         3468     3468           
  Misses         10       10

Impacted Files	Coverage Δ
src/num.c	`99.41% <100%> (ø)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 38cc514...96518d8. Read the comment docs.

gavinhoward commented 4 years ago

Yes, you did find a bug.

Your fix looks great, and your tests do as well. I just have two requests:

Move your tests and results into files called comp.txt and comp_results.txt.
Change the name in tests/bc/all.txt.

I want you to do this because I am going to write a more comprehensive set of tests for comparisons in general to make sure I did not miss anything else.

If you don't have the time to do this, I could always pull it and do the change myself.

eg15 commented 4 years ago

Just finished, thank you.

BTW it looks like busybox guys took your code and kept the bug in their modified version. Just fixed it there as well: https://bugs.busybox.net/show_bug.cgi?id=12336

Then it should be fixed in Alpine (that's where I encountered it this morning).

gavinhoward commented 4 years ago

busybox didn't exactly take my code; I gave them my code, when it had a lot more bugs as well. Unfortunately, they decided to go their own way with it, so I don't really contribute back. (They added even more bugs.)

I will merge this when I get home from work. I should have a release out after testing, which could take up to a week. I don't expect this to take that long to test, since it's so small, but I may find other problems.

gavinhoward commented 4 years ago

I pulled manually. Thank you for your help!