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

Issue with Currency #55

Closed
GoogleCodeExporter opened this Issue Mar 29, 2015 · 18 comments

Comments

Projects
None yet
1 participant
@GoogleCodeExporter

GoogleCodeExporter commented Mar 29, 2015

What steps will reproduce the problem?
1.Verify a class using a Currency field in its equals
2.Call Currency.getInstance(anyParameter)

What is the expected output? What do you see instead?
Currency.getInstance throws a StringIndexOutOfBoundsException

What version of the product are you using? On what operating system?
Version 1.0.2 on Ubuntu


I wonder if that could be fixed if you just add Currency to the list of java 
classes that can't be instantiated (in PrefabValuesFactory).
See junit test attached.


Original issue reported on code.google.com by dupont.n...@gmail.com on 2 Feb 2012 at 12:01

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

Definitely. Adding the following line in PrefabValuesFactory.addClasses fixes 
the issue :
prefabValues.put(Currency.class, Currency.getInstance("EUR"), 
Currency.getInstance("USD"));

Original comment by dupont.n...@gmail.com on 2 Feb 2012 at 12:08

  • Added labels: ****
  • Removed labels: ****
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

Hi Nicolas,

Thanks for your bug report!

However, I can't reproduce the error you describe. I've run the test you 
attached to this issue, and it passes.

Which Java version are you using? (I've tried using OpenJDK 6 and 7.) Also, 
maybe the error only occurs in conjunction with other unit tests that work on 
the same classes. I noticed that Currency has a few non-final static fields, so 
something similar to Issue 52 might be occurring.

Also, there's a small bug in your test code: the instanceof check should be for 
MyDomain, not Currency. But that doesn't seem to affect the results for this 
particular issue.

Original comment by jan.ouw...@gmail.com on 3 Feb 2012 at 8:08

  • Added labels: ****
  • Removed labels: ****
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

Hi Jan,

Thanks for your answer. I'm using Oracle JDK 1.6.0_30. It's not related to 
other tests, as I can reproduce the issue in a standalone Maven project using 
Maven 3.0.3. I've just attached the new Maven project.
Test fails with the following exception :
java.lang.StringIndexOutOfBoundsException: String index out of range: 147
    at java.lang.String.charAt(String.java:686)
    at java.util.Currency.getMainTableEntry(Currency.java:373)
    at java.util.Currency.getInstance(Currency.java:249)
    at EqualsCurrencyTest.test(EqualsCurrencyTest.java:14)

Original comment by dupont.n...@gmail.com on 3 Feb 2012 at 9:04

  • Added labels: ****
  • Removed labels: ****

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

Hi Nicolas,

I still can't reproduce the bug. I've tried it at home and at work (both on 
Ubuntu 11.10 though), and Maven happily reports a successful build.

I did notice 2 interesting differences though.
1. What does Locale.getDefault() return on your machine? This is the string 
that it's trying to do a charAt on. For me, the result is en_US. (Not that 
that's accurate though :).)
2. The line numbers in the stack trace are completely different for me, so we 
have different implementations of the Currency class, and therefore different 
JDKs. On my home machine, I have this one:

~/test$ java -version
java version "1.6.0_23"
OpenJDK Runtime Environment (IcedTea6 1.11pre) (6b23~pre11-0ubuntu1.11.10.1)
OpenJDK 64-Bit Server VM (build 20.0-b11, mixed mode)


Another thing that just occurs to me -- do you get the error also if you call 
Currency.getInstance before you call EqualsVerifier? If not, what is the locale 
when you do that? It's possible that EV also affects the Locale somehow, as a 
result of the problem described in Issue 52, and that your problem is caused by 
a default Locale that's somehow invalid.


Regards,
Jan

Original comment by jan.ouw...@gmail.com on 3 Feb 2012 at 11:05

  • Added labels: ****
  • Removed labels: ****
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

Hi Jan,
I tried with a constant locale (US), the issue remains :
Locale loc = Locale.US;
Currency.getInstance(loc); //OK
EqualsVerifier.forClass(MyDomain.class).suppress(Warning.NONFINAL_FIELDS).verify
();
Currency.getInstance(loc); //fails

Also, note that Currency.getInstance works fine when I call it before EV.

Also, it sounds like a JDK-specific issue. I use the JDK from Oracle :
java version "1.6.0_30"
Java(TM) SE Runtime Environment (build 1.6.0_30-b12)
Java HotSpot(TM) 64-Bit Server VM (build 20.5-b03, mixed mode)

Regards. Nicolas

Original comment by dupont.n...@gmail.com on 6 Feb 2012 at 9:38

  • Added labels: ****
  • Removed labels: ****
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

Hi Nicolas,

Thanks for your reply.

I've downloaded the Oracle JDK, and there I can indeed reproduce the problem. I 
have already determined that Issue 52 is not related to this issue, but I'll 
have to investigate further to find out what is. Of course I could just add 
Currency to the PrefabValuesFactory, as you suggested, but I'd like to know the 
cause of this issue, because it might happen with other classes as well.

In any case, I want to fix this for the next release. In the mean time, you can 
use withPrefabValues as a workaround. Or download OpenJDK :).


Jan

Original comment by jan.ouw...@gmail.com on 6 Feb 2012 at 1:55

  • Changed state: Accepted
  • Added labels: ****
  • Removed labels: ****
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

I've fixed this issue in version 1.1, which is available now!

I have also fixed the issue that EqualsVerifier didn't recognise that you were 
using the wrong instanceof check. (Currency instead of MyDomain; see comment 
#2.)

Original comment by jan.ouw...@gmail.com on 11 Feb 2012 at 11:52

  • Changed state: Fixed
  • Added labels: ****
  • Removed labels: ****
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

I have just tried writing a test using v1.1 and after the test is finished 
Currency.mainTable has the value of "one". This again ends up throwing an 
StringIndexOutOfBoundsException later on. Perhaps it needs to be reset?

Original comment by russell....@gmail.com on 23 Feb 2012 at 4:14

  • Added labels: ****
  • Removed labels: ****
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

Hi Russell,

Resetting fields, like Currency.mainTable, is exactly what I implemented in 
version 1.1, but maybe I overlooked something.
Unfortunately, I'm not able reproduce your problem. Could you tell me exactly 
which JVM you are using, and give me a piece of sample code that reproduces the 
problem?

Jan

Original comment by jan.ouw...@gmail.com on 23 Feb 2012 at 7:50

  • Added labels: ****
  • Removed labels: ****
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

Using jdk1.6.0_30 for my JRE. Have attached code that reproduces it for me and 
the resulting stack trace. Exception occurs in-between first and second test. 
Am looking for user error but so far still can't get it to work for me.

Original comment by russell....@gmail.com on 27 Feb 2012 at 3:03

  • Added labels: ****
  • Removed labels: ****

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

Hi Russell,

I still can't reproduce your problem. I don't have access to your Money class, 
so I'm not sure if that makes a difference somehow. If I replace it with 
java.util.Currency, both your tests pass.
Also, you mentioned getting a StringIndexOutOfBoundsException, but I don't see 
this in the stacktrace you attached. If you want to see a more detailed error 
message from EqualsVerifier, you can use the #debug() method to print out a 
more complete stacktrace.
Also, can you tell me if you're using OpenJDK, Oracle's JVM, or some other JVM?

Finally, I'm afraid I have to ask: are you really sure you updated to version 
1.1 (or 1.1.1)? If I revert back to 1.0.2, I do get a failing test, which may 
or may not be the one you refer to.


Jan

Original comment by jan.ouw...@gmail.com on 28 Feb 2012 at 8:26

  • Changed state: New
  • Added labels: ****
  • Removed labels: ****
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

Hi Jan. I've attached the Money object, added a clearer stack trace with the 
actual exception and added a screenshot of my test classpath. I updated to 
1.1.1 and also had a friend test these with these classes on a separate 
environment and he also gets the same problem. We are both using Oracle JVMs.


Original comment by russell....@gmail.com on 28 Feb 2012 at 11:00

  • Added labels: ****
  • Removed labels: ****

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

Hi Russell,

This is getting embarrassing :). I still can't reproduce the issue in Eclipse 
on an Oracle JVM. Also, I can't see anything wrong in the stuff you sent me.

Also, I'm using Ubuntu 11.11, and Eclipse 3.7. I notice you're using Windows, 
and I'm not sure which version of Eclipse. Although that shouldn't have 
anything to do with it...

In theory, it could still be something like the fact that you're using a 
slightly different version of JUnit. After all, you said the error occurs in 
between tests... Could you try running it with a different version of JUnit? 
4.8, for example?

If that doesn't give any results, I'd like to ask you to go one step further, 
and package your files in a completely self-contained Maven project. (If you 
don't know Maven, a self-contained Eclipse project will probably also do, as 
long as you include every single dependency, including JUnit.) That way, we can 
hopefully shake out any other dependency issues relatively quickly.


Jan

Original comment by jan.ouw...@gmail.com on 28 Feb 2012 at 12:53

  • Added labels: ****
  • Removed labels: ****
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

Hey Jan,

I know what you mean :). I will put together a maven project later but here is 
my Eclipse project as it is currently.

Russ

Original comment by russell....@gmail.com on 28 Feb 2012 at 2:54

  • Added labels: ****
  • Removed labels: ****

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

Hi Russ,

Good news: I was finally able to reproduce your problem! On my work machine the 
tests remained green, but at home, they finally turned red.
Haven't had time to investigate yet, but I'll let you know when I know more.

Jan

Original comment by jan.ouw...@gmail.com on 28 Feb 2012 at 5:15

  • Added labels: ****
  • Removed labels: ****
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

Ok, I found the problem.

EqualsVerifier did indeed reset all static fields for the class under test. It 
also reset the static fields for the fields of the class under test. But it did 
not reset the static fields for the fields of the fields of the class under 
test...

The reason why it didn't show up on my work machine, was that for some reason, 
it did not listen to me changing the JVM it should use. When I removed both 
OpenJDKs, it finally switched to the Oracle JVM, which has a slightly different 
implementation of java.util.Currency that causes the issue to appear.

I made a fix which I will probably release within the next few days or so.


Jan

Original comment by jan.ouw...@gmail.com on 29 Feb 2012 at 7:49

  • Changed state: Accepted
  • Added labels: ****
  • Removed labels: ****
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

I've just released the fix, in version 1.1.2!

Original comment by jan.ouw...@gmail.com on 1 Mar 2012 at 8:18

  • Changed state: Fixed
  • Added labels: ****
  • Removed labels: ****
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 29, 2015

Great news Jan, thx!

Original comment by russell....@gmail.com on 6 Mar 2012 at 3:26

  • Added labels: ****
  • Removed labels: ****

jqno pushed a commit that referenced this issue Aug 23, 2016

Merge pull request #55 from yihangho/master
Bump up the versions of Jekyll and other dependencies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment