Skip to content

Conversation

@danicheg
Copy link
Member

This PR aims to unify the test framework in cats-based projects in typelevel org.
It is worth mentioning - test running time have increased. I've tried to figure out a reason, but it needs more investigation.

Gen.choose(0, limit).flatMap(heapGen[Int](_, Arbitrary.arbitrary[Int]))

property("PairingHeap.combineAll (remove) does < log_2 N + 8 work") =
forAll(genPairingHeapLimitedSize(10)) { heap: PairingHeap[Int] =>
Copy link
Member Author

Choose a reason for hiding this comment

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

In the previous version based on Scalatest, the heap have generated with max size 10 (although there is no explicit limit in Gen[A] I saw it when the manually printing size of the heap). So, I've added the explicit limit on heap size because it often was a length bigger than 10 in the new version based on MUnit.

@danicheg
Copy link
Member Author

It'd be nice if you can check this @gemelen, @lukoyanov, @osleonard. We're getting closer to unify test-framework in typelevel projects (munit + scalacheck) and I suppose that is the right move.

@osleonard
Copy link
Collaborator

It'd be nice if you can check this @gemelen, @lukoyanov, @osleonard. We're getting closer to unify test-framework in typelevel projects (munit + scalacheck) and I suppose that is the right move.

Will take a look, later in the evening

@gemelen
Copy link
Collaborator

gemelen commented Aug 30, 2021

@danicheg I've already looked over, but stopped due lack of CI runs and was looking to fix that.
@osleonard are you able to run ci workflow manually (or to invoke it another way)?

@rossabaker
Copy link
Member

CI didn't run because @danicheg is a first-time contributor, and GitHub added the anti-mining protections. I'm not sure why that didn't show up as the typical "Approve and Run" button here. Anyway, I kicked it off, and you can see it here.

This will happen again if another commit is pushed to this PR, until @danicheg gets one merged. I think @osleonard and @gemelen would have permissions to kick it off. If not, I'll look deeper into why not.

@danicheg
Copy link
Member Author

danicheg commented Aug 30, 2021

It seems workflow needs to be regenerated. And then it's needed to run CI pipeline yet another time for this PR.

@gemelen
Copy link
Collaborator

gemelen commented Aug 31, 2021

I suggest to rebase this branch

@danicheg
Copy link
Member Author

I suggest to rebase this branch

Done. Though there were no conflicts.

@danicheg
Copy link
Member Author

danicheg commented Sep 3, 2021

It looks like there are no blockers to merge this. And there is a chance that will also fix #425.

@osleonard
Copy link
Collaborator

It looks like there are no blockers to merge this. And there is a chance that will also fix #425.

Yeap will merge this now.

@osleonard osleonard merged commit 413dca9 into typelevel:master Sep 3, 2021
@danicheg danicheg deleted the munit branch September 3, 2021 09:59
@danicheg
Copy link
Member Author

danicheg commented Sep 3, 2021

@osleonard thanks!

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