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

Bug fixes for directed edge #559

Merged
merged 35 commits into from
Jan 23, 2022

Conversation

isaacbrodsky
Copy link
Collaborator

Based upon #553. This updates the tests with some cases found via fuzzing to the test suite.

@coveralls
Copy link

coveralls commented Jan 13, 2022

Coverage Status

Coverage increased (+0.007%) to 98.149% when pulling 979ce07 on isaacbrodsky:llvm-fuzzer-directed-edge into 9330ee1 on uber:master.

@@ -329,6 +329,19 @@ SUITE(gridDisk) {
t_assert(out == origin, "Moving to self goes to self");
}

TEST(h3NeighborRotations_invalid) {
// This is undefined behavior, but it's helpful for it to make sense.
H3Index origin = 0x811d7ffffffffffL;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Anything special about this particular H3 index?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No - I just needed a known good index as I wanted the only rotations to be under test.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For my info, is there any difference between indexes with the L suffix or without? Should we be consistent?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All valid H3 indexes would only be valid integers as 64 bit, so no I don't believe so.

@@ -329,6 +329,19 @@ SUITE(gridDisk) {
t_assert(out == origin, "Moving to self goes to self");
}

TEST(h3NeighborRotations_invalid) {
// This is undefined behavior, but it's helpful for it to make sense.
H3Index origin = 0x811d7ffffffffffL;
Copy link
Collaborator

Choose a reason for hiding this comment

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

For my info, is there any difference between indexes with the L suffix or without? Should we be consistent?

@@ -335,6 +335,9 @@ H3Error h3NeighborRotations(H3Index origin, Direction dir, int *rotations,
H3Index *out) {
H3Index current = origin;

if (dir < CENTER_DIGIT || dir >= INVALID_DIGIT) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

For my info, what does CENTER_DIGIT even do? Returns the current index?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that is my expectation

@isaacbrodsky isaacbrodsky merged commit e91d79e into uber:master Jan 23, 2022
@isaacbrodsky isaacbrodsky deleted the llvm-fuzzer-directed-edge branch January 23, 2022 17:43
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.

None yet

4 participants