-
Notifications
You must be signed in to change notification settings - Fork 5
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
Tapiro core tests #211
Tapiro core tests #211
Conversation
@@ -0,0 +1,58 @@ | |||
package io.buildo.tapiro |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file is copy-pasted from Metals
almost verbatim
|
||
class TapiroSuite extends munit.FunSuite { | ||
|
||
check( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a test, the idea is that you pass in the controller code and the test runs tapiro and checks the expected code against the generated one.
|} | ||
|""".stripMargin, | ||
""" | ||
|/src/main/scala/schools/endpoints/SchoolControllerTapirEndpoints.scala |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line starting with a single /
declare files that are written to disk (in a temp dir)
bdbb0a9
to
cbb2f08
Compare
9aec302
to
b3ec027
Compare
import Formatter.format | ||
|
||
val logger = LogManager.getLogger("io.buildo.tapiro") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this does not show up when running the sbt plugin, I'll open a separate issue for it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
I've tried to rebase my PR #212 on this and add more tests (not pushed yet). I think it works quite nicely so I'd be in favour of merging this as is (bar the conflict to fix) and keep the "add more tests" part for #212 since tests on POST endpoints would have to be updated anyway. @gabro what do you think? @calippo did you get a chance to look at this? |
I'm 👍 for merging it as is. This needs a rebase, but it should be easy. |
I had a quick look, lgtm |
9cfce78
to
6a1c2fa
Compare
Rebased, we can merge as soon as the CI passes |
Based on #210, only the last commit is relevant.
TODO
add a NOTICE for the code I've copy-pasted from Metals