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

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

Open
zainab-ali opened this issue Nov 7, 2024 · 6 comments
Open

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

zainab-ali opened this issue Nov 7, 2024 · 6 comments

Comments

@zainab-ali
Copy link
Contributor

This issue was copied over from: disneystreaming/weaver-test#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
Copy link
Contributor Author

This comment was copied over from: disneystreaming/weaver-test#479 (comment)
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
Copy link
Contributor Author

This comment was copied over from: disneystreaming/weaver-test#479 (comment)
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
Copy link
Contributor Author

This comment was copied over from: disneystreaming/weaver-test#479 (comment)
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
Copy link
Contributor Author

This comment was copied over from: disneystreaming/weaver-test#479 (comment)
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
Copy link
Contributor Author

This comment was copied over from: disneystreaming/weaver-test#479 (comment)
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
Copy link
Contributor Author

This comment was copied over from: disneystreaming/weaver-test#479 (comment)
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.

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

1 participant