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

[SEDONA-249] Add jvm flags for running tests on Java 17 #772

Merged
merged 2 commits into from
Feb 18, 2023

Conversation

umartin
Copy link
Contributor

@umartin umartin commented Feb 17, 2023

Did you read the Contributor Guide?

Is this PR related to a JIRA ticket?

What changes were proposed in this PR?

Add jvm flags to test plugins to make it possible to run tests on Java 17. This will make life easier for contributors running Java 17 locally.

This PR does not add Java 17 to the test matrix for github actions.
This PR does not make the tests pass on Java 17. Currently CRSTransformationTest.testPolygonDistanceJoinWithCRSTransformation in sedona-core fails on Java 17.

How was this patch tested?

All tests pass on Java 8.

Did this PR include necessary documentation updates?

  • No, this PR does not affect any public API so no need to change the docs.

@jiayuasu
Copy link
Member

@umartin Thanks for bringing this up. Interestingly, I found CRSTransformationTest.testPolygonDistanceJoinWithCRSTransformation also failed on Java 11. I haven't figured why. Any idea?

@umartin
Copy link
Contributor Author

umartin commented Feb 17, 2023

I think there was a patch in a point release of Java 11 that changed the string representation of floats. For me it fails on Java 11.0.17 although I remember it passing on earlier version of Java 11. It the test reads geometries from csv files they could be interpreted differently depending on java version.

I remember tweaking some tests in sedona-sql when that change hit me. https://github.com/apache/sedona/blob/master/sql/src/test/scala/org/apache/sedona/sql/functionTestScala.scala#L163

The test runs a distance join in RDD with polygons. The Circle geometry is only valid for points. Maybe the test can be removed?

@jiayuasu
Copy link
Member

@umartin Yes! Can you remove that flaky test?

@jiayuasu jiayuasu added this to the sedona-1.4.0 milestone Feb 17, 2023
@umartin
Copy link
Contributor Author

umartin commented Feb 18, 2023

@jiayuasu No problem! I added a commit removing the test.

@jiayuasu jiayuasu merged commit edc6114 into apache:master Feb 18, 2023
gregleleu pushed a commit to gregleleu/sedona that referenced this pull request Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants