Skip to content

Bump to 4.5.0, add reverseDirectedEdge#207

Open
isaacbrodsky wants to merge 4 commits into
uber:masterfrom
isaacbrodsky:bump-4.5.0
Open

Bump to 4.5.0, add reverseDirectedEdge#207
isaacbrodsky wants to merge 4 commits into
uber:masterfrom
isaacbrodsky:bump-4.5.0

Conversation

@isaacbrodsky

@isaacbrodsky isaacbrodsky commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Updates to H3 v4.5.0, adds reverseDirectedEdge, and applies related test fixes.


note: we will need to update these tests:

> Task :test FAILED

TestRegion > h3SetToMultiPolygonSingle() FAILED
    org.opentest4j.AssertionFailedError at TestRegion.java:376

TestRegion > h3SetToMultiPolygonContiguous2() FAILED
    org.opentest4j.AssertionFailedError at TestRegion.java:425

TestRegion > h3SetToMultiPolygonSingleNonGeoJson() FAILED
    org.opentest4j.AssertionFailedError at TestRegion.java:402

TestRegion > h3SetToMultiPolygonSingle() FAILED
    org.opentest4j.AssertionFailedError at TestRegion.java:150

TestRegion > h3SetToMultiPolygonContiguous2() FAILED
    org.opentest4j.AssertionFailedError at TestRegion.java:199

TestRegion > h3SetToMultiPolygonSingleNonGeoJson() FAILED
    org.opentest4j.AssertionFailedError at TestRegion.java:176

274 tests completed, 6 failed, 3 skipped

@coveralls

coveralls commented Jun 7, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 637

Coverage increased (+0.008%) to 96.962%

Details

  • Coverage increased (+0.008%) from the base build.
  • Patch coverage: 2 of 2 lines across 1 file are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 572
Covered Lines: 561
Line Coverage: 98.08%
Relevant Branches: 185
Covered Branches: 173
Branch Coverage: 93.51%
Branches in Coverage %: Yes
Coverage Strength: 0.98 hits per line

💛 - Coveralls

@isaacbrodsky isaacbrodsky marked this pull request as ready for review June 8, 2026 20:09
@isaacbrodsky

Copy link
Copy Markdown
Collaborator Author

This job had to be run twice to get a green pass: https://github.com/uber/h3-java/actions/runs/27163934054/job/80188499610
The first run failed with SIGSEGV: https://github.com/uber/h3-java/actions/runs/27163934054/job/80186653134

...
[100%] Linking C shared library lib/libh3-java.so
+ popd
[100%] Built target h3-java
+ cp h3-java-build/build/binding-functions .
+ popd
~/work/h3-java/h3-java/target ~/work/h3-java/h3-java
+ true
~/work/h3-java/h3-java
+ exit 0

> Task :processResources
> Task :classes
warning: [options] source value 8 is obsolete and will be removed in a future release

> Task :compileTestJava

> Task :processTestResources NO-SOURCE
> Task :testClasses
warning: [options] target value 8 is obsolete and will be removed in a future release
warning: [options] To suppress warnings about obsolete options, use -Xlint:-options.
3 warnings
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  SIGSEGV (0xb) at pc=0x00007f3fc6eceee8, pid=2859, tid=2862
#
# JRE version: Java(TM) SE Runtime Environment Oracle GraalVM 21.0.11+9.1 (21.0.11+9) (build 21.0.11+9-LTS-jvmci-23.1-b92)
# Java VM: Java HotSpot(TM) 64-Bit Server VM Oracle GraalVM 21.0.11+9.1 (21.0.11+9-LTS-jvmci-23.1-b92, mixed mode, sharing, tiered, jvmci, jvmci compiler, compressed oops, compressed class ptrs, g1 gc, linux-amd64)
# Problematic frame:
# C  [ld-linux-x86-64.so.2+0x15ee8]
#
# Core dump will be written. Default location: Core dumps may be processed with "/lib/systemd/systemd-coredump %P %u %g %s %t 9223372036854775808 %h %d" (or dumping to /home/runner/work/h3-java/h3-java/core.2859)
#
# An error report file with more information is saved as:
# /home/runner/work/h3-java/h3-java/hs_err_pid2859.log
#
# If you would like to submit a bug report, please visit:
#   https://bugreport.java.com/bugreport/crash.jsp
# The crash happened outside the Java Virtual Machine in native code.
# See problematic frame for where to report the bug.
#
> Task :test FAILED
> Task :jacocoTestReport
gradle/actions: Writing build results to /home/runner/work/_temp/.gradle-actions/build-results/__run-1780949566036.json


FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':test'.
> Process 'Gradle Test Executor 1' finished with non-zero exit value 134
  This problem might be caused by incorrect test process configuration.
  For more on test execution, please refer to https://docs.gradle.org/8.14/userguide/java_testing.html#sec:test_execution in the Gradle documentation.

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
> Get more help at https://help.gradle.org./

BUILD FAILED in 58s
[Incubating] Problems report is available at: file:///home/runner/work/h3-java/h3-java/build/reports/problems/problems-report.html

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.14/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.
7 actionable tasks: 7 executed
Error: Process completed with exit code 1.

@isaacbrodsky isaacbrodsky requested review from ajfriend and dfellis June 9, 2026 01:03
Comment on lines +425 to +444
assertEquals(actualBounds.get(1).lat, multiBounds.get(0).get(0).get(8).lat, EPSILON);
assertEquals(actualBounds.get(2).lat, multiBounds.get(0).get(0).get(9).lat, EPSILON);
assertEquals(actualBounds.get(3).lat, multiBounds.get(0).get(0).get(0).lat, EPSILON);
assertEquals(actualBounds.get(4).lat, multiBounds.get(0).get(0).get(1).lat, EPSILON);
assertEquals(actualBounds.get(5).lat, multiBounds.get(0).get(0).get(2).lat, EPSILON);
assertEquals(actualBounds2.get(4).lat, multiBounds.get(0).get(0).get(3).lat, EPSILON);
assertEquals(actualBounds2.get(5).lat, multiBounds.get(0).get(0).get(4).lat, EPSILON);
assertEquals(actualBounds2.get(0).lat, multiBounds.get(0).get(0).get(5).lat, EPSILON);
assertEquals(actualBounds2.get(1).lat, multiBounds.get(0).get(0).get(6).lat, EPSILON);
assertEquals(actualBounds2.get(2).lat, multiBounds.get(0).get(0).get(7).lat, EPSILON);
assertEquals(actualBounds.get(1).lng, multiBounds.get(0).get(0).get(8).lng, EPSILON);
assertEquals(actualBounds.get(2).lng, multiBounds.get(0).get(0).get(9).lng, EPSILON);
assertEquals(actualBounds.get(3).lng, multiBounds.get(0).get(0).get(0).lng, EPSILON);
assertEquals(actualBounds.get(4).lng, multiBounds.get(0).get(0).get(1).lng, EPSILON);
assertEquals(actualBounds.get(5).lng, multiBounds.get(0).get(0).get(2).lng, EPSILON);
assertEquals(actualBounds2.get(4).lng, multiBounds.get(0).get(0).get(3).lng, EPSILON);
assertEquals(actualBounds2.get(5).lng, multiBounds.get(0).get(0).get(4).lng, EPSILON);
assertEquals(actualBounds2.get(0).lng, multiBounds.get(0).get(0).get(5).lng, EPSILON);
assertEquals(actualBounds2.get(1).lng, multiBounds.get(0).get(0).get(6).lng, EPSILON);
assertEquals(actualBounds2.get(2).lng, multiBounds.get(0).get(0).get(7).lng, EPSILON);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is super-brittle. Could we instead confirm that the Set of multiBounds is the same as actualBounds + actualBounds2 instead? Then we won't have to rework this test if there are other changes to how the boundary is calculated.

assertEquals(actualBounds.size() + 1, multiBounds.get(0).get(0).size());

int[] expectedIndices = {0, 1, 2, 3, 4, 5, 0};
int[] expectedIndices = {3, 4, 5, 0, 1, 2, 3};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same nit here: if the order doesn't match the input ordering anymore, we should compare by Set instead of an array.

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

Successfully merging this pull request may close these issues.

3 participants