Issue11 wsdl2rest integration #31
Conversation
|
To run the generator with the additional wsdl2rest functionality, now you simply type: This will prompt you for two additional properties:
Unfortunately at this point, I'm not successful in getting the wsdl2rest utility to run in this manner. I see this output locally: So this hints that a few things -- our logging path is off for some reason (though it correct) and the utility is not taking the passed-in details though they are specified in the command line. I'm a bit stumped here, but at least we're making some progress. Thank you @apupier for the fat-jar fixes! |
|
We can add some additional command-line options as well for the passed-in wsdl URL and the path for the generated Java files, but for now I thought we'd just focus on the prompt path. |
|
Working on adding additional Mocha tests now... |
|
I've asked a question on StackOverflow to see what shakes out. I'm sure it's something simple - https://stackoverflow.com/questions/53105600/how-do-i-build-a-mocha-test-that-waits-for-a-process-to-finish-first |
bfitzpat
force-pushed
the
issue11-wsdl2rest-integration
branch
from
e5c493c to
f1b5921
Compare
bfitzpat
added
DO NOT MERGE
|
Now that we have a working test I can complete the work to get additional prompts in for optional properties. (Presuming that the test also works up on Travis) |
bfitzpat
force-pushed
the
issue11-wsdl2rest-integration
branch
from
f1b5921 to
fff8c23
Compare
|
Looks like mocha is failing with a timeout. I will investigate upping the timeout in the travis configuration. |
bfitzpat
added
READY FOR REVIEW
and removed
DO NOT MERGE
There was a problem hiding this comment.
- Can you clean the commit history please? at least providing the combine the camel-project and wsdl2rest generators into one composed generator #11 in the commit messages so that we know to which issue all the commits are related to
- I think it would worth to update the readme.md - Development notes as there are new steps required with maven module included (aren't they?)
- I think that the springboot jar would be a best fit in wsdl2rest repo directly but I guess it will take too much time to integrate it in this other repo and it will give "productization" constraints. Otherwise, maybe it could be in a separate repository to avoid mixing npm and maven build system? What do you think? Having it inside this repo is the best? Or enough for a first iteration?
- I know that the mocha test was giving hard time. I'm not sure in the test what was the missing piece to make it working? Can you explain me so that I won't hit the same wall when I will write a mocha test myself please?
will try it locally tomorrow for reviewing from a functional point of view but wanted to share initial feedback before
bfitzpat
force-pushed
the
issue11-wsdl2rest-integration
branch
4 times, most recently
from
590592b to
ad41322
Compare
|
Due to some funky issues I hit with a merge of readme.md, I will handle updating the readme separately with #32 |
|
@apupier -- to answer your list of questions
First, there was an issue with how the wsdl2restGenerate method was calling the Java jar. Thanks to some help from Evan Shortiss, he recommended returning a Promise -- with either a resolve (success) or reject (error), and then relying on that in the test using "toPromise" to wait for the promise to resolve itself before attempting to do the asserts. I was missing the return of the Promise from the method upstream, which meant it really didn't have anything to wait for in my test. |
|
Once this particular PR is merged I will move on to #12 to finish the work. |
…ng#31 Signed-off-by: Brian Fitzpatrick <bfitzpat@redhat.com>
There was a problem hiding this comment.
- why do we have stderr message?
stderr: log4j:WARN No appenders could be found for logger (org.jboss.fuse.wsdl2rest.impl.WSDLProcessorImpl).
stderr:
log4j:WARN Please initialize the log4j system properly.
log4j:WARN See http://logging.apache.org/log4j/1.2/faq.html#noconfig for more info.
No way to fix that easily?mvn
-
if I understand well, it is generating always a Camel project and if --wsdl2rest is set, it is also generating the rest dsl file with some classes. The rest route is not used at all by default, I fear that it will lead users to confusion. The difference with the approach in Eclipse Fuse Tooling is that we are generating only the rest on an already existing Camel project which is making it more "expectable" that users will need to modify the project to take the new route into account. What do you think?
-
the generated classes are in a src/main/java/java folder. It seems that there is an extra java folder hierarchy
-
I think the summary at the end is misleading, it not listing the camel rest route, neither the Java files generated related to rest. it is mentioning only the "other artefacts" with a green "create" message.
|
For now I have "skipped" the test that is not working to test that we can build, run, and access the running rest service. We will continue to work on figuring out the issues with the test, but it should not block us from getting this functionality out to our users. We can then focus on improving that functionality as we go forward. |
bfitzpat
force-pushed
the
issue11-wsdl2rest-integration
branch
from
fbfac82 to
b765c4a
Compare
|
i have errors with blueprint wsdl2rest when trying to run the blueprint project: |
will the tests now detect a potential regression? |
|
The Mocha/Yeoman framework we are using does not handle validating each prompt as we go through them, but I can likely write a test for the potential regression. Will see what I can do about that and Blueprint. |
bfitzpat
force-pushed
the
issue11-wsdl2rest-integration
branch
from
b765c4a to
aef98db
Compare
|
@apupier Added a test for the camel dsl validation (with and without wsdl2rest). And dropped a note to Freeman and Claus about the Blueprint issue. So far as I can tell, the pom is being created with all the required blueprint dependencies to get Felix to run. Wonder if it's a versions issue with camel? |
|
I got some feedback from Freeman about running the generated Blueprint example in Standalone mode. It seems some changes would need to be made to make that happen. See this new issue I added to wsdl2rest - jboss-fuse/wsdl2rest#73 @apupier If you want to test the generated project locally, you can follow those steps to get it to run standalone and then do the same "http://localhost:8081/jaxrs/hello/%22Aurelien%22" test on it to pass through Camel to the SOAP service and back again. So this will require some upstream changes to make it work for Blueprint in a testable capacity. What's there now should work for a deployable case (deploying to a Karaf instance), but won't work for a standalone test. As a result, I think we can call this done for Spring, but state that Blueprint will require some additional work. |
agree. i think if we put a comment in readme for now, it will be enough to move forward with this PR on this point. |
|
there is a test which is not finishing https://travis-ci.org/camel-tooling/generator-camel-project/builds/462869962#L4053-L4058 it is blocked when locally launching "npm test", I guess it says "pending" on CI because there is a timeout set in the package.json EDIT: hum it seems that it is because the test is marked as "skip" so don't understand why it blocks on my machine. √ Should create the basic structure
|
|
Very strange. It never gets stuck here. And it's not getting stuck on CI. Maybe do "npm link" locally to clean up any lingering code from older builds and try "npm test" again? |
|
Are you sure you have rebased it? I only have |
17 is from the CI
|
yes, using "npm test", I would expect it to work :( Edit: I have the same blocked test with 'mocha', also after tryign to "npm install" and "npm link" |
|
My bad on the number of tests - I forgot I added a new test on my other machine and didn't sync this one. Now I get: That said, I still have no clue why it's failing on your box. Is it possible you have a Java process that's hanging somewhere? |
there are not. I don't understand why another java process would hang the test execution. |
There was a problem hiding this comment.
I would have preferred to not put the skipped test in this PR to simplify the review.
I'm not able to launch test successfully locally anymore. It still hangs. i tried to comment several tests and it seems to happen for several of them. Consequently, I wasn't able to validate that the non-regression test about validation are working.
it is working as expected when testing manually and CI is reporting success.
it would be nice to start splitting code in different files, see #35
I have had instances where the Terminal inside (or outside) VSCode has caused a hung Java process on my Windows box. Just wanted to suggest that as a possibility. Thanks for checking. |
bfitzpat
force-pushed
the
issue11-wsdl2rest-integration
branch
from
aef98db to
a33432d
Compare
-- initial cut with fat jar and running the wsdl2rest tool with inputs -- updating to support full paths and a logging.props file -- Updating version for when we include wsdl2rest functionality -- also updating the travis file to build the wsdl2rest jar -- additional work on test and wsdl2rest call -- additional fixes -- Setting jar to specific version vs asterisk and returning to broken test -- updates to make wsdl2rest run in a promise camel-tooling#11 -- Increasing the mocha timeout in the travis configuration camel-tooling#11 -- addressing some review feedback -- updating tests to run with npm test call -- reset readme.md -- updating console messages to better reflect what's happening to user -- adding --debug flag for wsdl2rest -- fixing log4j issue -- adding appropriate dependencies to pom for wsdl2rest projects -- fixing issue when using URL to non-local WSDL -- adding test for non-local WSDL -- adding validation to avoid java and wsdl2rest combo -- adding function to avoid hardcoding wsdl2rest fat jar version -- addressing more feedback -- fixing test for jar finding function -- increased mocha test timeout to 15000 -- added new test that starts a sample JAX-WS web service and uses the WSDL it serves up to create a project -- updated helloworld wsdl and sample WS -- added jaxws and jaxrs URL property prompts -- fixed helloworld wsdl to generate properly -- added new command line arguments and first test -- removed some extraneous console messages and debug settings -- improved wsdl2rest command line tests -- adjusted to ensure that all wsdl2rest command line args are present -- fixed issue with java dsl option -- addressed more minor feedback -- added test-in-progress to run generated service and do a get request against it -- more test tweaks -- attempting child_process -- fixing issue camel-tooling#34 (missing camel-maven-plugin version for spring projects) -- fixing error with test soap utility -- adding test for camel dsl validation -- split up tests and did some test cleanup Signed-off-by: Brian Fitzpatrick <bfitzpat@redhat.com>
bfitzpat
force-pushed
the
issue11-wsdl2rest-integration
branch
from
a33432d to
dbb4e68
Compare
|
@apupier I have split up the test into three pieces -- generator_tests, wsdl2rest_tests, and utils_tests -- and removed the maven/chai testing that was skipped. Can you give it a shot again? |
…ng#31 -- adding wsdl2rest details and known issues Signed-off-by: Brian Fitzpatrick <bfitzpat@redhat.com>
-- adding wsdl2rest details and known issues Signed-off-by: Brian Fitzpatrick <bfitzpat@redhat.com>
Incorporates all the latest changes to master as well as the changes proposed by @apupier for a "fat jar" version of wsdl2rest for the generator.
Signed-off-by: Brian Fitzpatrick bfitzpat@redhat.com