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 Thread Context ClassLoader to find test resources #1607

Merged

Conversation

holly-cummins
Copy link
Contributor

Hi! I'm working on improving Pact support in Quarkus (see quarkusio/quarkus#27729). At the moment, Pact provider tests works well when executed via a normal mvn verify, but do not work in Quarkus's dev mode. The reason is that in dev mode the classpath is less flat, and so it's no longer safe to assume the PactLoader is in the same classloader as the test classes.

Pact can be made more tolerant to non-flat-classloader hierarchies by switching to use the Thread context classloader for finding resources. This should also improve behaviour in other frameworks.

@holly-cummins
Copy link
Contributor Author

As an alternative implementation, we could look use the Pactloader's classloader, and then fallback to the thread context classloader if there's nothing found using Pact's loader; or the other way round.

@rholshausen
Copy link
Contributor

All the things I can find about the Thread context classloader online say not to use it, but that is for class loading (makes sense, because if the thread changes, then what classes are accessible can change). I think it is safe for test resources, because the thread should not change during the execution of a test.

@rholshausen rholshausen merged commit 7258f43 into pact-foundation:master Sep 19, 2022
@rholshausen
Copy link
Contributor

Now that I think about it, the problem may be that it is using PactFolderLoader::class.java.classLoader, when it should be using getClass().getClassLoader().

@holly-cummins
Copy link
Contributor Author

Thanks for the fast merge, @rholshausen!

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.

2 participants