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

Handle Karate tests in a BootJar #833

Merged
merged 1 commit into from
Jul 26, 2019
Merged

Conversation

celcius112
Copy link
Contributor

@celcius112 celcius112 commented Jul 19, 2019

Hello, so after some investigation here's half (or maybe 2/3) of the solution to launch Karate tests inside a jar packaged by Spring Boot.

Here are the important changes brought by this PR:

  • In the Runner, the featureExecutor will be initialized using a privilegedThreadFactory() in order to have access to the invoking thread's contextClassLoader. Spring Boot sets its own ClassLoader in the thread's context, and this ClassLoader is very useful in retrieving packaged feature files. Each ExecutionContext retrieves the contextClassLoader, which is then passed down to each ScenarioContext.
  • Across scenarios, this ClassLoader might be null. This is handled by passing the initial one through a CallContext. When resolving a scenario we cannot retrieve the Spring Boot's ClassLoader by using Thread.currentThread().getContextClassLoader() because we are in a Thread managed by a ForkJoinPool (the executor scenarioExecutor in Runner).
  • This ClassLoader retrieves the karate-http.properties
  • The ClassLoader is used in the FileUtils to create a Resource wrapping a URL. This URL is a good pointer to the packaged files.
  • The test class BootJarLoadingTest uses the newly added jar karate-bootjar-test.jar to test the new modifications.

I previously said "half of the solution" because at the moment this and relative path resolution mechanisms do not work, such as call read('this:a.feature') and call read('../a.feature'). Also, the Karate's Resource class should probably be modified in order to be more generic in the resources it can handle (see BootJarLoadingTest to see the working solution we've been using since we started using Karate with Spring Boot). I'm currently working on a solution for that.

@celcius112
Copy link
Contributor Author

And thank a lot for all these existing tests 😅 It makes me more confident in my changes

@ptrthomas
Copy link
Member

@celcius112 this is good stuff and I have no problem merging this. but should I wait for the "full" solution ?

@celcius112
Copy link
Contributor Author

@ptrthomas I don't mind waiting. This PR was done in order to have more insight (if needed) on the changes before moving on the next issues. Anyway we won't use these changes in production until Karate is released

@ptrthomas
Copy link
Member

@celcius112 okay, let's wait - as you said, since all the existing tests pass we should be good, thanks for writing tests and do add more if needed

@ptrthomas
Copy link
Member

@celcius112 just one thought - if we move the ClassLoader reference into the Resource class would that make things a little simpler ?

@celcius112
Copy link
Contributor Author

yeah I though about exploring this solution for simplifying the Resource

@celcius112 celcius112 changed the title Handle Karate tests in a BootJar WIP : Handle Karate tests in a BootJar Jul 22, 2019
@celcius112 celcius112 changed the title WIP : Handle Karate tests in a BootJar Handle Karate tests in a BootJar Jul 22, 2019
@celcius112
Copy link
Contributor Author

Hello @ptrthomas, this and relative path resolution mechanisms now work correctly. If you accept I think this commit is now mergeable and answer to our current need.

For your interest, I will try in the near future to modify the FileUtils#scanForFeatureFiles so that it can detect more easily the feature files in a bootjar. Right now we need to add some specific implementation in order to create our own Resource for Spring Boot resources (see BootJarLoadingTest#SpringBootResourceLoader).

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