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

Re-encode relationships to avoid implicit conversion functions #3323

Merged
merged 1 commit into from
Feb 27, 2020

Conversation

travisbrown
Copy link
Contributor

This change only affects test code that I added recently, so it should be fairly uncontroversial.

In #3307 I introduced some abstraction for NonEmptyX testing, since there were a lot of inconsistencies and gaps not just in the interfaces themselves, but also in their tests.

In that change I encoded the relationship between the non-empty type and the type that has its operations as a couple of implicit parameters:

  ev: NE[Int] => NEC[Int],
  evPair: NE[(Int, Int)] => NEC[(Int, Int)]

I couldn't use <:< because for NonEmpty types that use the newtype encoding, NE isn't actually a subtype of NEC (e.g. NonEmptyChain and NonEmptyChainOps).

The issue is that this encoding in the tests doesn't work on Dotty, and in any case is a little inelegant—if for example we want to work with additional NE[x] values in the future, we'd need new evX parameters here.

I've changed NonEmptyCollectionSuite to use a new abstract method instead:

protected def toNonEmptyCollection[A](nea: NE[A]): NEC[A]

This fixes the tests on Dotty and provides a bit of extra future-proofing even on Scala 2.

@codecov-io
Copy link

Codecov Report

Merging #3323 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3323   +/-   ##
=======================================
  Coverage   93.31%   93.31%           
=======================================
  Files         378      378           
  Lines        7684     7684           
  Branches      233      233           
=======================================
  Hits         7170     7170           
  Misses        514      514
Flag Coverage Δ
#scala_version_212 93.38% <ø> (ø) ⬆️
#scala_version_213 93.09% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e12f4d...ed4611c. Read the comment docs.

@travisbrown travisbrown merged commit 23de622 into typelevel:master Feb 27, 2020
@travisbrown travisbrown deleted the topic/follow-up-3307 branch February 27, 2020 15:05
scala-steward pushed a commit to scala-steward/cats that referenced this pull request Feb 27, 2020
@travisbrown travisbrown added this to the 2.2.0-M1 milestone Feb 29, 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.

4 participants