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

Don't skip PartialOrder antisymmetry and transitivity #3493

Merged

Conversation

rossabaker
Copy link
Member

PartialOrderTests' transitivity and antisymmetry properties are shadowed by EqTests'. When discipline finds duplicate laws in parent rulesets, the child's properties are silently dropped. This counterintuitive behavior and sent us on a wild chase on http4s/http4s#3518.

We work around this by changing the EqTests' names to match the law names.

@rossabaker
Copy link
Member Author

Example output now. Note the eq-suffixed properties:

[info] - UpperBounded[UUID].upperBounded.antisymmetry (1 millisecond)
[info] - UpperBounded[UUID].upperBounded.antisymmetry eq (1 millisecond)
[info] - UpperBounded[UUID].upperBounded.bound is greater than or equals (1 millisecond)
[info] - UpperBounded[UUID].upperBounded.gt (2 milliseconds)
[info] - UpperBounded[UUID].upperBounded.gteqv (1 millisecond)
[info] - UpperBounded[UUID].upperBounded.lt (1 millisecond)
[info] - UpperBounded[UUID].upperBounded.partialCompare (1 millisecond)
[info] - UpperBounded[UUID].upperBounded.pmax (1 millisecond)
[info] - UpperBounded[UUID].upperBounded.pmin (1 millisecond)
[info] - UpperBounded[UUID].upperBounded.reflexivity eq (1 millisecond)
[info] - UpperBounded[UUID].upperBounded.reflexivity gt (1 millisecond)
[info] - UpperBounded[UUID].upperBounded.reflexivity lt (1 millisecond)
[info] - UpperBounded[UUID].upperBounded.symmetry eq (1 millisecond)
[info] - UpperBounded[UUID].upperBounded.transitivity (2 milliseconds)
[info] - UpperBounded[UUID].upperBounded.transitivity eq (1 millisecond)

I ran with a modified version of discipline, and antisymmetry and transitivity were the only clashes I identified.

@codecov-commenter
Copy link

Codecov Report

Merging #3493 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3493      +/-   ##
==========================================
+ Coverage   91.74%   91.77%   +0.02%     
==========================================
  Files         383      383              
  Lines        8399     8399              
  Branches      218      208      -10     
==========================================
+ Hits         7706     7708       +2     
+ Misses        693      691       -2     

Copy link
Member

@LukaJCB LukaJCB left a comment

Choose a reason for hiding this comment

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

good catch!

@LukaJCB LukaJCB merged commit e256ab3 into typelevel:master Jun 21, 2020
@travisbrown travisbrown added this to the 2.2.0-RC1 milestone Jun 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants