karatelabs / karate

Test Automation Made Simple
https://karatelabs.github.io/karate
MIT License
8.32k stars 1.95k forks source link

Issue with Karate in an application packaged by Spring Boot #751

Closed celcius112 closed 3 years ago

celcius112 commented 5 years ago

Hi @ptrthomas, I'm back with more issues with classpath, Spring Boot, and JAR (or bootJAR in this case).

I have a dummy project here, which is a Spring Boot application that launches a single runner class (com.karate.karatespring.feature.Runner.java) on startup, using the JUnit 5 test factory. This runner class will simply launch the feature file Feature.feature that asserts on a boolean initialized in karate-config.js.

Problems arise when I launch the JAR. The feature file is found 45 times by FileUtils.scanForFeatureFiles, and as such is launched 45 times (which is a bit too much). Also, the karate-config.js file is not found on the classpath, with the exception skipping bootstrap configuration: could not find or read file: classpath:karate-config.js.

I found these issues by migrating from version 0.8.x to version 0.9.x.

Thank you for your time, and thank you for your work on Karate.

ptrthomas commented 5 years ago

tagged as "help wanted" you know what to do @celcius112 ;)

celcius112 commented 5 years ago

That was quick 😛 I've already tried (quickly) to find a solution, but since the project is quite large in my standards I might need some time to find a solution that does not break anything 😁

ptrthomas commented 5 years ago

take all the time you need, since no one else has reported problems since #520 - and you know how hard that was

warrenc5 commented 5 years ago

even just some nice debug logging so we can see exact file it's looking for and where it's looking.

image

ptrthomas commented 5 years ago

closing because of inactivity

ptrthomas commented 5 years ago

@celcius112 awesome work on the PR ! re-opening as "fixed" :) this will be in 0.9.5

ptrthomas commented 5 years ago

@celcius112 turns out some of the changes completely broke things on windows. I have had to make some changes and comment out your spring-boot test which now fails

it is back to the drawing board a little - and I request you to test on windows before submitting the next PR. I would also try using the URL as a last resort if really no other way to get the stream works.

celcius112 commented 5 years ago

that is some pretty horrendous news 😢 I'll probably need to rework everything

ptrthomas commented 5 years ago

@celcius112 no it is fine, I really appreciate your contribution - very few people do. I did do some more improvements to the FileUtils and I don't think you need to re-work much. as I said, I don't really like to depend on a URL in the Resource instance, but if that is the only thing that will work for spring-boot, please go ahead and add it. but try to follow the existing pattern (maybe cache the contents in the static Map like you already see for JAR resources) - maybe I asked you already, but if you want to join a slack channel send me an e-mail: https://github.com/intuit/karate/wiki/Support

ptrthomas commented 5 years ago

@celcius112 closing due to inactivity

joelpramos commented 4 years ago

@celcius112 I'm facing the same (exactly like @ptrthomas warned in #1225 ...) and wondering whether you ever got to work around this

ptrthomas commented 4 years ago

@joelpramos can you create the smallest possible JAR or project where we can see this problem - I'll take a look sometime. in the meantime - do try and debug and make sense of the FileUtils code

celcius112 commented 4 years ago

hello @joelpramos, the issue came from the different Classloader used by Kareta versus the one used by the spring-loader when launching a bootjar. I have created a PR to resolve this issue, but unfortunately some rework was necessary in order to make it work on windows, which I was far from motivated to do (and I'm sorry for that 😞 ).

We have found another solution, which is to not use a bootjar for our application. Previously, we were putting this fat jar in a docker image, but now we instead layer the image by using jib.

joelpramos commented 4 years ago

@ptrthomas I've been trying to go through FileUtils and the Resource file (my current exception is on the method Resource.getStream() so looking at changes there for now). I didn't fully understand all the changes @celcius112 did in his PR but will keep going through it today for inspiration :)

Example here: https://github.com/joelpramos/karate-classpath

The latter is what I'm trying to achieve in my build - external feature files but have a config file and extensions/helper methods pre-loaded.

joelpramos commented 4 years ago

Got a fix for the 2nd problem (doesn't seem to address the first) here: https://github.com/joelpramos/karate/commit/8b506060cdb62c38dbd7ddaab9575be81aaa5867 (there are several comments and System.out.println() just wanted to show you).

Found another problem with the way I'm using Karate in which Nashorn can't load a class from the Spring Boot classpath (this is a pretty good hint .. https://github.com/spring-projects/spring-boot/issues/2823) so I'll be looking into that.

@ptrthomas both of these might not be exactly "bugs" but symptoms of the way I'm using Karate - not in the unit tests running in the machine but packaged in a .jar file in the app. Are you good if I continue to provide updates here? Also let me know if you want me to do separate PRs if/when I find a fix for this additional problem.

ptrthomas commented 4 years ago

@joelpramos this helps - sorry for not replying yesterday. I'm actually in the middle of a super-ambitious re-write of the core and moving things over to GraalVM refer #1281 - and one of the things I'm keen to do is get the file / classloading right this time. so once I get a chance I plan to look through your log and make sure the new code works and retro-apply whatever possible to develop in the meantime

joelpramos commented 4 years ago

@ptrthomas no problem - this is open source I don't expect an immediate answer :) To workaround the Nashorn issue I exposed a method to allow me to instantiate the Nashorn engine directly. I tried with passing thread context etc but seems like when the parallel runner starts the classpath context is lost. With that new method all I do is after starting my spring applications I call the following line: ScriptBindings.initNashorn(SpringApplication.class.getClassLoader());

Not very pretty but should also only be use for very specific usages of Karate (or maybe just me .... )

I don't think either change impacts the GraalVM refactor. I ran through the unit tests with no issuess and tested the change in Windows - not sure if there's any unforeseen behavior on Linux... In any case for the main change to solve the first issue I tackled has defensive coding - in case there's no file found using the classLoader.getResourceAsStream() it falls back to the previous implementation (Files.newInputStream()).

Let me know if you're ok with these changes and I'll open the PR. Here's the diff: https://github.com/intuit/karate/compare/develop...joelpramos:bugfix/callSingle-in-karate-basejs-from-classpath?expand=1

ptrthomas commented 4 years ago

@joelpramos ok. just one thing - on the rewrite branch I made some changes to Resource and FileUtils - mainly I did not want Resource to need a ClassLoader passed any more and have only one (less confusing) entry point. I would like to add these changes to the develop branch before you proceed, or would you be able to experiment first along those lines to confirm

here's one diff: https://github.com/intuit/karate/commit/b27854e5b098fac2052a4bb89a5136216b24db6b#diff-cea3f582a9c266b663188079ae4981b9

also a question for you is that if everything starts out using ClassLoader.getSystemClassLoader() is that a better approach

the PR certainly looks fine, just want to take time to think through this

joelpramos commented 4 years ago

I'll try to merge your changes to those two files to minimize conflicts and test it on my end.

I tried using getSystemClassLoader() and it fails (see here). I haven't found an issue with the thread contextloader() so I'd suggest to stick with that. According to this stackoverflow thread the class itself typically has the correct classloader so I'll probably try that as well. I was a bit inconsistent with it myself as I used the class classloader in HttpClient but not on Resource so I'll review that too.

ptrthomas commented 4 years ago

@joelpramos great, thanks - I agree that in any case we should expose a method via the API so that one can force-override a custom class-loader at run-time

ptrthomas commented 4 years ago

@joelpramos I have reworked the resource / classpath scanning code using io.github.classgraph - would be great if you can check if this works in spring boot now

joelpramos commented 3 years ago

@ptrthomas is this in the RC for 1.0.0 or is the rewrite branch still separate? Looking to start migrating / testing really soon.

ptrthomas commented 3 years ago

@joelpramos yes in the RC and everything is now in develop also refer: https://github.com/intuit/karate/wiki/1.0-upgrade-guide

joelpramos commented 3 years ago

Closing the loop - I didn't notice any of my previous issues, tried with feature files in classpath, executed from intellij, compile the jar and execute externally, as well as feature files externalized from the .jar file itself and found no issues with classpath scanning. Seems to be working fine @ptrthomas . Thanks!

ptrthomas commented 3 years ago

@joelpramos that is pretty excellent news, thanks 👍

ptrthomas commented 3 years ago

1.0 released