search

typelevel / weaver-test

A test framework that runs everything in parallel.
https://typelevel.org/weaver-test/
Other
46 stars 8 forks source link

Unable to resolve GlobalResource defined in a submodule/project #101

Open zainab-ali opened 5 days ago

zainab-ali commented 5 days ago

This issue was copied over from: https://github.com/disneystreaming/weaver-test/issues/479 It was opened by: aevanszen

If you define a GlobalResource in a subproject/module for re-use, i.e. a test-support-module, the GlobalResource cannot be resolved/used in a test in another module. You will get the below sample output:

[info] MyTestSuite
[info] - Unexpected failure 0ms
[info] *************FAILURES**************
[info] MyTestSuite
[error] - Unexpected failure 0ms
[error]   GlobalResourceF$ResourceNotFound: Could not find a resource of type java.lang.String with label null
[error]
[error]   GlobalResourceF.scala:110    weaver.GlobalResourceF$Read#$anonfun$getOrFailR$1
[error]   Stream.scala:3475            fs2.Stream$#$anonfun$resourceWeak$3
[error]   Stream.scala:3475            fs2.Stream$#$anonfun$resourceWeak$3$adapted
[error]   Stream.scala:1191            fs2.Stream$#$anonfun$flatMap$1
[error]   Algebra.scala:620            fs2.internal.FreeC$#$anonfun$flatMapOutput$1
[error]   Algebra.scala:53             fs2.internal.FreeC$$anon$1#cont
[error]   Algebra.scala:240            fs2.internal.FreeC$ViewL$$anon$9$$anon$10#<init>
[error]   Algebra.scala:240            fs2.internal.FreeC$ViewL$$anon$9#cont
[error]   Algebra.scala:234            fs2.internal.FreeC$ViewL$$anon$8#next
[error]   Algebra.scala:475            fs2.internal.FreeC$#$anonfun$compile$8
[error]   CECompat.scala:51            eval @ weaver.CECompat#resourceLift
[error]   CompileScope.scala:413       map @ fs2.internal.CompileScope#interruptibleEval
[error]   Algebra.scala:503            flatMap @ fs2.internal.FreeC$#go$1
[error]   Algebra.scala:463            flatMap @ fs2.internal.FreeC$#$anonfun$compile$7
[error]   Algebra.scala:460            flatMap @ fs2.internal.FreeC$#go$1
[error]   Algebra.scala:436            flatMap @ fs2.internal.FreeC$#interruptGuard$1
[error]   Algebra.scala:436            flatMap @ fs2.internal.FreeC$#interruptGuard$1
[error]   CompileScope.scala:185       flatMap @ fs2.internal.CompileScope#$anonfun$acquireResource$4
[error]   ScopedResource.scala:139     flatten @ fs2.internal.ScopedResource$$anon$1#acquired
[error]   CompileScope.scala:183       flatMap @ fs2.internal.CompileScope#$anonfun$acquireResource$1
[error]   CompileScope.scala:180       flatMap @ fs2.internal.CompileScope#acquireResource
[error]   Algebra.scala:511            flatMap @ fs2.internal.FreeC$#$anonfun$compile$12
[error]   Algebra.scala:436            flatMap @ fs2.internal.FreeC$#interruptGuard$1
[error]   Algebra.scala:436            flatMap @ fs2.internal.FreeC$#interruptGuard$1
[error]   Algebra.scala:436            flatMap @ fs2.internal.FreeC$#interruptGuard$1
[error]   CompileScope.scala:185       flatMap @ fs2.internal.CompileScope#$anonfun$acquireResource$4

I have a sample project demonstrating this issue at https://github.com/aevanszen/weaver-global-resource-classloading-bug

It would be desirable to use GlobalResources defined in other subprojects/libraries for common code re-use.

A workaround is to define the GlobalResource as a trait in the shared project and then define an object extending the trait in the project you want to use the GlobalResource. This feels very much like a workaround, not desired functionality; for every project/subproject, you wish to re-use a common GlobalResource, you need to define a local class to the module by extending the shared trait.

zainab-ali commented 5 days ago

This comment was copied over from: https://github.com/disneystreaming/weaver-test/issues/479#issuecomment-1042671269 It was written by: Baccata

I'm sorry to say that's not something we can solve at the level of weaver. Weaver does not do any crawling through the classpath to retrieve global resource instances on its side, it's the build tool that is responsible for detecting the suites from the classes resulting from the compilation of the test scope, and then pass it to the test framework.

I understand it's somewhat confusing, the relationship/delimitation between test framework and build tool is not obvious to the users. It's the exact same thing that causes the issue raised by your colleague there.

Weaver is only responsible for telling the build tool "please look for classes extending this particular interface when you crawl through the test classpath", which happens here.

You could most likely solve it at the level of the build tool (SBT), by setting something so that SBT would would look further than the immediate classes, but I'm afraid I'm not aware of how to do it. Maybe keynmol would be able to give pointers though, his knowledge of SBT is better than mine.

zainab-ali commented 5 days ago

This comment was copied over from: https://github.com/disneystreaming/weaver-test/issues/479#issuecomment-1043546344 It was written by: aevanszen

Thank you for the quick reply.

I'll investigate the SBT settings, and if I find anything, I'll report back and raise a PR to add a note to the GlobalResources docs.

zainab-ali commented 5 days ago

This comment was copied over from: https://github.com/disneystreaming/weaver-test/issues/479#issuecomment-1044048171 It was written by: aevanszen

I understand what is happening a little better after investigating.

When you call SBT test, it passes in all compiled classes from the project test folder as a Task for each item to Weaver. Weaver then looks at the task and decides what to do with that class, i.e. is it a GlobalResource, is it a suite etc.

As Weaver relies on the GlobalResource detection from the passed in SBT task, it currently doesn't know about any GlobalResource instances on the classpath outside of the current SBT project.

Looking a bit further, Weaver could use Java's SPI (Service Provider Interface) to detect additional GlobalResource's on the classpath. We can then merge the instances detected using SPI into the GlobalResources collected from SBT TaskDefs.

In src-jvm/RunnerCompat, we can use the below sample to detect additional GlobalResources from other submodules/dependencies to merge into our list:

import scala.jdk.CollectionConverters._
val spiGlobalResourceLoader: ServiceLoader[GlobalResourceF[F]] = ServiceLoader.load(classOf[GlobalResourceF[F]])
val spiGlobalResources: List[GlobalResourceF[F]] = spiGlobalResourceLoader.iterator().asScala.toList

Then in subprojects, the user/library owner creates the file src/main/resources/META-INF/services/weaver.GlobalResourceF. Inside that file, they list the GlobalResource's they want to expose using the fully qualified class name.

Here are some docs on using Java SPI for reference https://docs.oracle.com/javase/tutorial/ext/basics/spi.html.

It looks relatively simple from a library perspective to support, we call ServiceLoader.load(classOf[GlobalResourceF[F]]) and merge the results into the existing global resources we get from the below:

val globalResources = tasksAndSuites.collect {
  case (_, suiteLoader.GlobalResourcesRef(init)) => init
}.toList

I can look into it a bit further and put a PR together if it is something the project would consider. I have a hacked together version running locally that looks to work.

zainab-ali commented 5 days ago

This comment was copied over from: https://github.com/disneystreaming/weaver-test/issues/479#issuecomment-1044063916 It was written by: Baccata

I'd be willing to accept an SPI based solution that would complement the current state. I'm very familiar with the mechanism, and I think the idea has legs 😸

I'll be happy to review a PR :)

zainab-ali commented 5 days ago

This comment was copied over from: https://github.com/disneystreaming/weaver-test/issues/479#issuecomment-1044340051 It was written by: keynmol

I think the idea has legs

But you also like custom classloaders and various other dangerous things :P

I've come to tentatively accept ServiceLoader approach because it's used in mdoc and it works there reasonably well. As long as we keep GlobalResourceF interface stable (like, really stable), then perhaps it can be made a "if you know what you are doing" option

zainab-ali commented 5 days ago

This comment was copied over from: https://github.com/disneystreaming/weaver-test/issues/479#issuecomment-1044351564 It was written by: Baccata

Any test framework that uses the SBT test interface has to deal with classloading, one way or another, it's not like we'd be stepping in a totally new danger zone.

Also, SPIs are basically the standard solution for any plug-able architecture on JVM. For instance, compiler plugins in Scala are loaded via SPIs. It's just the most sane way to do this kind of thing.

That being said, maybe rather than using Service loaders directly, the plugin mechanism could just read the metadata file (like ServiceLoader does) from JVM resources, and construct the correct fingerprint instance.