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

Test Suites that use class loader/reflection fail on second run #547

Closed
rgueldem opened this issue Jul 12, 2022 · 4 comments
Closed

Test Suites that use class loader/reflection fail on second run #547

rgueldem opened this issue Jul 12, 2022 · 4 comments

Comments

@rgueldem
Copy link

rgueldem commented Jul 12, 2022

We are using weaver for some internal libraries and found that some test suites succeed only on the first run and always fail on the second run. This impacts test suites that depend on Java libraries that use reflection, in our case testcontainers and slf4j. I have created a minimal project that can be used to reproduce the issue.

sbt:Scala 3 Project Template> test
[info] compiling 1 Scala source to /Users/rgueldemeister/Code/rgueldem/weaver-demo/target/scala-3.1.3/test-classes ...
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
SLF4J: Failed to load class "org.slf4j.impl.StaticMDCBinder".
SLF4J: Defaulting to no-operation MDCAdapter implementation.
SLF4J: See http://www.slf4j.org/codes.html#no_static_mdc_binder for further details.
[info] MySuite
[info] + example test that succeeds 10s
[info] Passed: Total 1, Failed 0, Errors 0, Passed 1
[success] Total time: 16 s, completed Jul 11, 2022, 9:18:47 PM
sbt:Scala 3 Project Template> test
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
[info] MySuite
[info] - example test that succeeds 20ms
[info] *************FAILURES**************
[info] MySuite
[error] - example test that succeeds 20ms
[error]   ServiceConfigurationError: org.testcontainers.dockerclient.DockerClientProviderStrategy: org.testcontainers.dockerclient.EnvironmentAndSystemPropertyClientProviderStrategy not a subtype
...

I am using testcontainers for the example, since that might be a common scenario. It can be reproduced with other dependencies as well. Happy to provide additional information to help with getting the issue resolved.

PS: Test / fork := true can be used to work around the issue for the time being.

@Baccata
Copy link
Contributor

Baccata commented Jul 12, 2022

I'm not sure I'd qualify this as a bug on weaver's side. Anything that plays with classloaders can be a little bit tricky when run through SBT in non-forked environments. I think it's pretty likely you'll get similar problems with other test frameworks. Have you tried this with other frameworks ?

In particular, it's a good idea for libraries that rely on classloaders to let the user inject a custom classloader, to ensure that they can avoid polluting the main-thread one. To elaborate : what I think is happening is that the MySQLContainer construct is mutating a classloader that belongs to SBT (by dynamically loading things into it), and that classloader has a longer lifecycle than your tests (because non-forked).

@rgueldem
Copy link
Author

rgueldem commented Jul 12, 2022

We are migrating a larger test suite for a multi-project build from scalatest to weaver (thank you for all your hard work, we greatly appreciate it). The tests are running in non-forked mode with scalatest. I don't know if there is something in sbt or scalatest that makes the class loader work correctly or if there is something in weaver that breaks it, but the behavior is definitely different. Running the tests in a fork works for us, but it increases the total time for all tests from ~40s to over 2 minutes.

@Baccata
Copy link
Contributor

Baccata commented Jul 12, 2022

Okay. I'm afraid I'm gonna lack the time to dive into it and the actual expertise to pinpoint the problem without diving into it. I can offer a wild guess :

Weaver is built for multi-threading. The testcontainer library does not allow to inject a classloader, so it relies on the one acquired from ThreadLocal, which probably doesn't play well with weaver's paradigm. I think that if testcontainer allowed to inject a classloader, the problem would be easier to solve.

Alternatively, you could try and see whether creating a fresh classloader here and passing it down, using the one received from the parameter as a classloader parent, would solve the issue. But it's a longshot.

I'm sorry you're experiencing this issue 😞

@rgueldem
Copy link
Author

Closing this issue since forking works fine.

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