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

Add JUnit Pioneer and use it in ServerErrorHealthCheck #54

Closed
sleberknight opened this issue Dec 10, 2020 · 4 comments · Fixed by #58
Closed

Add JUnit Pioneer and use it in ServerErrorHealthCheck #54

sleberknight opened this issue Dec 10, 2020 · 4 comments · Fixed by #58
Assignees
Labels
code cleanup Fix issues reported by Sonar or any other code analysis tools
Milestone

Comments

@sleberknight
Copy link
Member

See my original comment here: #53 (comment)

@sleberknight sleberknight added the code cleanup Fix issues reported by Sonar or any other code analysis tools label Dec 10, 2020
@sleberknight sleberknight self-assigned this Dec 10, 2020
@sleberknight sleberknight added wontfix This will not be worked on and removed code cleanup Fix issues reported by Sonar or any other code analysis tools labels Dec 11, 2020
@sleberknight
Copy link
Member Author

sleberknight commented Dec 11, 2020

Added Pioneer and then attempted to use @DoubleRangeSource in ServerErrorHealthCheckTest. Alas, got reminded why floating point math sucks, because we do calculations with the input values to mock out the rate for the Meter, and we include the approximate count in the health check details. In any case you end up with errors like this:

java.lang.AssertionError: [Expected detail not found] 
Expecting map:
 <{"approximateCount"=0.8999999999999998, "criticalThreshold"=10, "meter"="io.dropwizard.jetty.MutableServletContextHandler.5xx-responses", "rate"=9.999999999999998E-4, "severity"="OK", "warningThreshold"=1}>
to contain:
 <[MapEntry[key="approximateCount", value=0.8999999999999999]]>
but could not find the following map entries:
 <[MapEntry[key="approximateCount", value=0.8999999999999999]]>

Without changing the code in the health check, this cannot work. For example it would work if instead we only included the first few digits in the details. But in order to do that with a double, we'd need to either use BigDecimal or use something like String.format("%.1f", value).

Do we want to do this? For example using .withDetail("approximateCount", String.format("%.1f", estimatedErrorCount)) for the above value (0.8999999999999998) would then result in a detail of ("approximateCount", 0.9)

@sleberknight sleberknight removed the wontfix This will not be worked on label Dec 11, 2020
@sleberknight
Copy link
Member Author

Actually the above is wrong...the detail's value would be the String "0.9" rather than a number. So we'd have to convert it back to a double to be semantically correct.

@sleberknight
Copy link
Member Author

sleberknight commented Dec 11, 2020

So it would need to be:

.withDetail("approximateCount", Double.parseDouble(String.format("%.1f", estimatedErrorCount)))

There's probably some much more sophisticated math way to do this, but...

@sleberknight sleberknight added the code cleanup Fix issues reported by Sonar or any other code analysis tools label Dec 11, 2020
@sleberknight
Copy link
Member Author

Decided to go ahead with this change, using the code above to set the approximateCount detail.

@sleberknight sleberknight added this to the 0.1.0 milestone Dec 11, 2020
sleberknight added a commit that referenced this issue Dec 11, 2020
* Update ServerErrorHealthCheck to show only one decimal place in the
  approximateCount result detail
* Add JUnit Pioneer 1.1.0 with exclusions since they are only on
  Jupiter 5.5.2 instead of 5.7.0
* Change ServerErrorHealthCheckTest to use Pioneer's @DoubleRangeSource
* Move kiwi-test dependency to its correct order below the "j" deps

Closes #54
Closes #57
chrisrohr pushed a commit that referenced this issue Dec 12, 2020
* Update ServerErrorHealthCheck to show only one decimal place in the
  approximateCount result detail
* Add JUnit Pioneer 1.1.0 with exclusions since they are only on
  Jupiter 5.5.2 instead of 5.7.0
* Change ServerErrorHealthCheckTest to use Pioneer's @DoubleRangeSource
* Move kiwi-test dependency to its correct order below the "j" deps

Closes #54
Closes #57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code cleanup Fix issues reported by Sonar or any other code analysis tools
Projects
None yet
1 participant