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

more detailed Applicability results / add metric to ConstraintResult / scoverage #76

Merged
merged 8 commits into from
Nov 20, 2018

Conversation

tdhd
Copy link
Contributor

@tdhd tdhd commented Nov 15, 2018

Description of changes:

  • each ConstraintResult in CheckResult is now linked to the metric that the constraint evaluation was based on
  • the result of Applicability.isApplicable(check) now contains all constraints and their applicabilities
  • adds scoverage with optional code coverage requirements, currently we are at a method coverage of 81% (run with mvn scoverage:report, https://github.com/scoverage/scoverage-maven-plugin)
  • test for constraint assertions that throw exceptions

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

val constraintApplicabilities = check.constraints.zip(namedMetrics).map {
case (constraint, (_, metric)) =>
constraint -> metric.isSuccess
}.toMap
Copy link
Contributor

Choose a reason for hiding this comment

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

.toMap should be on the next line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still haven't lost hope for myself that I'll learn deequ style :)

Do you think we could have a rule for it in our style check?

Copy link
Contributor

Choose a reason for hiding this comment

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

We tried to configure such rules, unfortunately, there is always an edge case that is not coverable by the stylechecker...

@sscdotopen sscdotopen merged commit 4a686b4 into master Nov 20, 2018
@sscdotopen sscdotopen deleted the phschmid/constraints branch November 20, 2018 12:44
@sscdotopen
Copy link
Contributor

@stefan-grafberger Thank you for having a look the changes!

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

3 participants