Skip to content

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.


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.


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?

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.

4 participants