-
Notifications
You must be signed in to change notification settings - Fork 87
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Big decimal range #69
Conversation
|
||
assertNotNull("Generated BigDecimal must not be null", bigDecimal); | ||
assertTrue("Generated value must be a BigDecimal", bigDecimal instanceof BigDecimal); | ||
assertTrue(new BigDecimal("1").compareTo(bigDecimal) != 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe here should be better to check if the generated value is in the range instead of checking if it's not the limit values
. Something like:
assertTrue(new BigDecimal("1").compareTo(bigDecimal) >= 1);
assertTrue(new BigDecimal("1000").compareTo(bigDecimal) <= 1000);
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback.
Yeah I agree, reading BigDecimal comparison is really annoying. But the problem is that the compareTo method will only return (-1, 0, 1) for less than, equals to, greater than the other value.
But I'll come up with some clever idea to make it a little more readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@csrcordeiro actually is possible to compare exactly the same as with operators, check the Javadoc here: http://docs.oracle.com/javase/7/docs/api/java/math/BigDecimal.html#compareTo(java.math.BigDecimal)
You could change your asserts to be like this:
assertTrue(bigDecimal.compareTo(new BigDecimal("1")) >= 0);
assertTrue(bigDecimal.compareTo(new BigDecimal("1000")) <= 0);
The above code is the same as:
assertTrue(number >= 1);
assertTrue(number <= 1000);
@csrcordeiro Travis CI ran all tests and nothing failed. I also ran the tests on my machine and the tests you mentioned worked:
Maybe it's a bug? What's your environment? Java version etc. I only added two little comments about the tests. |
@nykolaslima Here are my environment settings:
The cause is a little bit odd, it seems to be in bytecode level so might be a OS issue:
I'll try to take a look at the problem a little further. |
@csrcordeiro about this |
Ok, I'll just fix the tests assertions. |
Thanks @csrcordeiro |
Hi Fixture Factory community,
This pull request is regarding issue #61.
To Implement this, I had to change both the BigDecimal and BigInteger features but I believe it will be ok since I was able to write tests for both cases. I also noticed a little typo in README tutorial so I fixed it.
There are tests failing in master branch but I couldn't figure out what it was about so I didn't fixed it, the tests are: