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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ModellingMonetaryAmountsTest.testImplementComparable does not cover the specification #18

Closed
oboehm opened this issue Aug 24, 2018 · 3 comments

Comments

@oboehm
Copy link

oboehm commented Aug 24, 2018

For Comparable only two notices appears in section 4.2.2.:

  • must be comparable
  • Comparison methods for comparing two arbitrary amounts of the same currency, hereby comparing
    based on the (effective) numeric value (e.g. ignoring trailing zeroes).

When I run the TCK I get the following error report:

[FAILED]  4.2.2 For each amount class, test is Comparable.(ModellingMonetaryAmountsTest#testImplementComparable):
java.lang.AssertionError: Section 4.2.2: Comparable failed for: de.jfachwert.bank.Geldbetrag
	at org.testng.AssertJUnit.fail(AssertJUnit.java:59)
	at org.testng.AssertJUnit.assertTrue(AssertJUnit.java:24)
	at org.javamoney.tck.tests.ModellingMonetaryAmountsTest.testImplementComparable(ModellingMonetaryAmountsTest.java:2522)
        ...

The input values for failed tests are:

  • amount = "0 XXX"
  • amount3 = "1 CHF"

The failing assert is assertTrue(amount.compareTo(amount3) > 0), or in other words

"0 XXX" > "1 CHF" should be true.

This is not covered by the specification (see above - or did I miss something?) and also from the semantic view I would expect it the other way around.

@marschall
Copy link
Member

I believe the section of the spec you're referring to refers to the # isGreaterThan #isGreaterThanOrEqualTo, #isLessThan, #isLessThanOrEqualTo and #isEqualTo methods. I understand these methods are supposed to operate on the same currency and throw MonetaryException otherwise.
It is my understanding #equals and #compareTo are supposed to work on different currencies and are not supposed to throw MonetaryException.

@keilw
Copy link
Member

keilw commented Nov 4, 2019

@atsticks Could you have a look and share your thoughts? If any update to the TCK was required, it would be good doing it with the MR1.

@keilw
Copy link
Member

keilw commented May 4, 2020

Seems fixed please reopen or create again, if you still face the problem with 1.1

@keilw keilw closed this as completed May 4, 2020
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

No branches or pull requests

3 participants