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

Use both Discipline and MutableIOSuite #528

Closed
bwiercinski opened this issue Jun 29, 2022 · 10 comments
Closed

Use both Discipline and MutableIOSuite #528

bwiercinski opened this issue Jun 29, 2022 · 10 comments

Comments

@bwiercinski
Copy link
Contributor

is it possible to use shared resource from MutableIOSuite inside checkAll from Discipline addon?

@Baccata
Copy link
Contributor

Baccata commented Jun 29, 2022

Not currently. I think it'd be a tad weird, in that typeclasse laws (which discipline is meant for) should probably not depend on some dynamic context.

But you have my curiosity, can you elaborate on the usecase ?

@bwiercinski
Copy link
Contributor Author

I've created a trait of Repository. I would like to make laws based on it. This requires connection to a database (e.g. Postgres run inside TestContainers).

I would disagree with that. If I would like to test if Slick's DBIO or Doobie's ConnectionIO passes laws of a Sync, how would I do that?

@Baccata
Copy link
Contributor

Baccata commented Jun 29, 2022

Mmm yeah, that's a fair point I guess. In this case I'd be happy to review a PR adding this feature 😄

@bwiercinski
Copy link
Contributor Author

grrr 😄

how to achive this? currently Discipline trait have a self-type of FunSuiteAux (trait Discipline { self: FunSuiteAux => ... })

obviously i have to use something that relays on MutableFSuite (which is a abstract class). should i create trait Discipline2 which have trait DisciplineIT[F[_]] { self: MutableFSuite[F] => ... }?

if i would like to edit current Discipline trait it would be binary incompatible although the users would only need to change:

+object FooLawTests extends FunSuite with Discipline {
-object FooLawTests extends SimpleMutableIOSuite with Discipline {

  checkAll("foo", FooTests(implementation).foo)

+  pureTest("if someone added any unit test) { ... }
-  test("if someone added any unit test) { ... }
}

@Baccata
Copy link
Contributor

Baccata commented Jun 30, 2022

Yeah let's try to make a new trait instead (could call it DisciplineEffect or whatever), in order to avoid breaking current usecases. Not that I think there are many of the, but still.

@bwiercinski
Copy link
Contributor Author

bwiercinski commented Jul 8, 2022

I'm in a crossroad right now. At first I wanted to use trait DisciplineEffect[F[_]] { self: MutableFSuite[F] => ... }. Its not so easy:
In standard suite all tests are collected using MutableFSuite.registerTest. If after filtering number of tests is 0 then there is no need to allocate resource.

In case of law testing I want to define following function checkAll(name: String, input: Res => F[Laws#RuleSet]) (one Laws#RuleSet contains many tests). So I want to firstly open a resource and then register tests, because without the resource I dont know how many tests there will be.

1. I can modify MutableFSuite to contain instead of

+var testSeq = Seq.empty[(TestName, Res => F[TestOutcome])]
-var testSeq = Seq.empty[Res => F[List[(TestName, TestOutcome)]]]

I'm losing benefit of not opening a resource in some cases. Is that something to concern or hardly ever used feature?

2. I can modify MutableFSuite to contain instead of

+var testSeq = Seq.empty[(TestName, Res => F[TestOutcome])]
-var testSeq = Seq.empty[Either[(TestName, Res => F[TestOutcome]),Res => F[List[(TestName, TestOutcome)]]]]

Complicated, but i can achieve the feature

In both cases I'm unable to create def plan: List[TestName]

3. Create Suite only for Discipline testing

Will all features work well? (eg. Global resource, filtering etc) Who knows :)

@Baccata
Copy link
Contributor

Baccata commented Jul 8, 2022

the def plan: List[TestName] is required in order for users to benefit from the out-of-the-box JUnit integration in Intellij.

I think this is sufficient to justify that it should be its own Suite, as opposed to extending the existing MutableFSuite. The most abstract thing you can extend is this, and I'd probably start there, as you'll be able to go 100% Pure FP.

  • Global resources should work without you having to do anything. As long as the user creates their suites classes with the same constructor as documented, I'm pretty confident it will just work.
  • You'll have to handle filtering yourself

@bwiercinski
Copy link
Contributor Author

I've created a draft #548. could you take a look if it makes sense, so i will go forward creating some tests

@bwiercinski
Copy link
Contributor Author

without runnable suite I'm unable to run it via intellij or sbt

@Baccata
Copy link
Contributor

Baccata commented Jul 12, 2022

without runnable suite I'm unable to run it via intellij or sbt

Oh, my bad, I take it EffectSuite is the minimum requirement. See here

Baccata added a commit that referenced this issue Jul 29, 2022
@Baccata Baccata closed this as completed Jul 29, 2022
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

No branches or pull requests

2 participants