-
Notifications
You must be signed in to change notification settings - Fork 59
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
Fix compilation with too long constant strings #26
Conversation
Length calculation is based on modified UTF-8. Reference: https://github.com/openjdk/jdk/blob/a8a2246158bc53414394b007cbf47413e62d942e/src/java.base/share/classes/java/io/DataOutputStream.java#L351-L358 Reference: https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.4.7 Fixes #25.
Codecov Report
@@ Coverage Diff @@
## master #26 +/- ##
============================================
- Coverage 89.53% 0.00% -89.54%
============================================
Files 47 47
Lines 1978 1996 +18
Branches 329 336 +7
============================================
- Hits 1771 0 -1771
- Misses 123 1996 +1873
+ Partials 84 0 -84 Continue to review full report at Codecov.
|
Interesting, Windows seems to have different constant pool sizes. I'll have a look.
|
Nice, thanks for PR @kelunik! I'll try to hop over to my windows PC and have a look at the failing tests. |
Running the tests on Windows via IntelliJ results in no errors. Will try maven... |
@kelunik okay when I run the tests via maven on Windows all multibyte offset tests fail here as well. Maybe some difference in JVM startup parameters compared to maven? |
…n windows maven build.
It seems that unicode output on Windows via maven doesn't give the desired output at all. Rendering a single Mr Poo works fine via IntelliJ launcher but fails when executed by maven surefire plugin. I committed the test to see if it is the case on Github Action, too.
|
Interesting, the GitHub Action says:
|
…ck-up the project encoding, being UTF-8)
Applying this argline to the surefire plugin worked for me locally, let's see what the Github Action is doing. |
I just committed the same change. 😄 |
@casid What's the default encoding on your Windows JVM? |
@kelunik this seems to be |
I can reproduce it locally with |
Nice! I've added one more test, just for the sake of coverage and would merge the PR after the build ran through. Thanks for fixing this @kelunik :-) |
Length calculation is based on modified UTF-8.
References:
Fixes #25.