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.

Sign up for GitHub

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

Jump to bottom

Change OpenApiGeneratorCodeGenBase#shouldRun to throw an exception rather than return false #192

Closed
hbelmiro opened this issue Dec 2, 2022 · 5 comments · Fixed by #314
Closed

Change OpenApiGeneratorCodeGenBase#shouldRun to throw an exception rather than return false #192

hbelmiro opened this issue Dec 2, 2022 · 5 comments · Fixed by #314
Labels
added to backlog The issue was added to backlog enhancement New feature or request

Comments

@hbelmiro
Copy link
Contributor

hbelmiro commented Dec 2, 2022

Silently returning false in io.quarkiverse.openapi.generator.deployment.codegen.OpenApiGeneratorCodeGenBase#shouldRun may confuse the user of the extension, that may not notice that an invalid path is being used.
It will be clearer if we throw an exception instead, so the build will fail.

This should be done after #179 is merged.
@hbelmiro hbelmiro added the enhancement New feature or request label Dec 2, 2022
@hbelmiro hbelmiro mentioned this issue Dec 2, 2022
Merged
hbelmiro added a commit to hbelmiro/issues-i-can-help-you-with that referenced this issue Dec 2, 2022
@hbelmiro
Added quarkiverse/quarkus-openapi-generator#192
e7febcc 
Signed-off-by: Helber Belmiro <helber.belmiro@gmail.com>
@hbelmiro hbelmiro changed the title Change OpenApiGeneratorCodeGenBase#shouldRun to throw exception rather Change OpenApiGeneratorCodeGenBase#shouldRun to throw an exception rather than return false Change OpenApiGeneratorCodeGenBase#shouldRun to throw an exception rather than return false Dec 2, 2022
hbelmiro added a commit to hbelmiro/issues-i-can-help-you-with that referenced this issue Dec 2, 2022
@hbelmiro
Added quarkiverse/quarkus-openapi-generator#192
7f54c91 
Signed-off-by: Helber Belmiro <helber.belmiro@gmail.com>
@hbelmiro hbelmiro added the added to backlog The issue was added to backlog label Dec 15, 2022
brunobaiano added a commit to brunobaiano/quarkus-openapi-generator that referenced this issue Apr 24, 2023
@brunobaiano
quarkiverse#192 Change OpenApiGeneratorCodeGenBase#shouldRun to throw…
2339a5a 
… an exception rather than return fals
@brunobaiano
Copy link
Contributor

I have a strange behavior in integrated-tests after implementing this:
I have to create a lot of openapi folders inside src/test
Is this an expected behavior?
You can check the modification here:

brunobaiano@2339a5a

The other file is commented because I'm not sure how to add a provider yet. (and now I have an error too)

@hbelmiro
Copy link
Contributor Author

@brunobaiano it is actually the bug that originated this issue. See this comment.
It happens because the tests try to find the openapi directory in src/test. It should try to find in src/main.
So, the PR to fix this issue has to change OpenApiGeneratorCodeGenBase#shouldRun to not return false without having to create the directories under src/test.

brunobaiano added a commit to brunobaiano/quarkus-openapi-generator that referenced this issue Apr 25, 2023
@brunobaiano
quarkiverse#192 add input-base-dir path on properties to generate-cod…
db74b9e 
…e-tests step
@brunobaiano
Copy link
Contributor

It's not returning false, it's throwing the exception. The thing is, all tests will fail now.
I can fix adding quarkus.openapi-generator.codegen.input-base-dir=src/main/openapi
on each application.properties in integration-tests. The change-directory test has this property and works.
you can see here brunobaiano@db74b9e

@hbelmiro
Copy link
Contributor Author

Sorry, @brunobaiano. I might have not expressed myself properly.

The PR to fix this issue needs to address two requirements:

  1. OpenApiGeneratorCodeGenBase#shouldRun can't return false at all. It must return only true or throw an exception. That's what you already did.
  2. The integration tests have to pass without you having to manually create the directory or set input-base-dir=src/main/openapi in application.properties. When users use this extension they shouldn't need to do that. Therefore, in integration tests, this mustn't be necessary either. Fixing it is part of the solution to this issue.

brunobaiano added a commit to brunobaiano/quarkus-openapi-generator that referenced this issue Apr 28, 2023
@brunobaiano
Revert "quarkiverse#192 add input-base-dir path on properties to gene…
5ff95e3 
…rate-code-tests step"

This reverts commit db74b9e.
brunobaiano added a commit to brunobaiano/quarkus-openapi-generator that referenced this issue Apr 28, 2023
@brunobaiano
quarkiverse#192 add condition to test path
113d99a
@brunobaiano
Copy link
Contributor

Hi again @hbelmiro,
I tried to find a good field in Config, but I couldn't find anything to help.
I create a PR with a solution, if you think it's a valid one, please let me know.

@hbelmiro hbelmiro linked a pull request Apr 28, 2023 that will close this issue
#192 Change OpenApiGeneratorCodeGenBase#shouldRun to throw an exception rather than return false #314
Merged
hbelmiro pushed a commit that referenced this issue Apr 28, 2023
@brunobaiano
#192 Change OpenApiGeneratorCodeGenBase#shouldRun to throw an except…
84060f8 
…ion rather than return false (#314)

* #192 Change OpenApiGeneratorCodeGenBase#shouldRun to throw an exception rather than return fals

* #192 add input-base-dir path on properties to generate-code-tests step

* Revert "#192 add input-base-dir path on properties to generate-code-tests step"

This reverts commit db74b9e.

* #192 add condition to test path
hbelmiro added a commit to hbelmiro/issues-i-can-help-you-with that referenced this issue Apr 28, 2023
@hbelmiro
Marked quarkiverse/quarkus-openapi-generator#192 as done
8fa0a25
hbelmiro pushed a commit to hbelmiro/quarkus-openapi-generator that referenced this issue Apr 28, 2023
@brunobaiano @hbelmiro
quarkiverse#192 Change OpenApiGeneratorCodeGenBase#shouldRun to thro…
8f58a4d 
…w an exception rather than return false (quarkiverse#314)

* quarkiverse#192 Change OpenApiGeneratorCodeGenBase#shouldRun to throw an exception rather than return fals

* quarkiverse#192 add input-base-dir path on properties to generate-code-tests step

* Revert "quarkiverse#192 add input-base-dir path on properties to generate-code-tests step"

This reverts commit db74b9e.

* quarkiverse#192 add condition to test path

(cherry picked from commit 84060f8)
@hbelmiro hbelmiro mentioned this issue Apr 28, 2023
Merged
hbelmiro added a commit that referenced this issue Apr 28, 2023
@hbelmiro @brunobaiano
#192 Change OpenApiGeneratorCodeGenBase#shouldRun to throw an except…
68c05c2 
…ion rather than return false (#314) (#316)

* #192 Change OpenApiGeneratorCodeGenBase#shouldRun to throw an exception rather than return fals

* #192 add input-base-dir path on properties to generate-code-tests step

* Revert "#192 add input-base-dir path on properties to generate-code-tests step"

This reverts commit db74b9e.

* #192 add condition to test path

(cherry picked from commit 84060f8)

Co-authored-by: Bruno Alves <brunobaiano@users.noreply.github.com>
@laurentperez laurentperez mentioned this issue May 17, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
added to backlog The issue was added to backlog enhancement New feature or request
Projects
None yet
Milestone
No milestone
2 participants
@hbelmiro @brunobaiano