Conversation
Extract server setup from main() into server/setup.go for each of the 9 variants (chi, gorilla, stdhttp, strict, gin, echo, echo-v5, fiber, iris). Each setup.go exports a factory function (NewServer, NewEchoServer, NewFiberApp, or NewIrisApp) that loads the swagger spec, creates the router, adds validation middleware, and registers handlers — returning the configured server/app and an error instead of calling os.Exit(). Replace per-variant petstore_test.go files with integration/main.go programs that start the server on a random port, exercise all CRUD operations via the shared testclient.Run(), and shut down cleanly. During this work we discovered that several server variants (gin, fiber, iris, and others) had server implementations that were not actually compatible with the generated HTTP client — the prior unit tests sidestepped this by testing against the handler directly with testutil rather than going through real HTTP. The shared test client exposed these gaps, which were fixed in earlier phases of this refactoring. Each petstore.go is now a thin wrapper: parse flags, call the factory, set the address, and serve. The old NewGinPetServer, NewFiberPetServer, and NewIrisPetServer factory functions that lived in the main package (and called os.Exit on error) have been replaced by proper error- returning functions in the server package. Also removes the echo/pkg_codegen_petstore_test.go codegen test and cleans up the now-unused golang.org/x/lint and testutil dependencies. README.md updated to document the new structure. The stdhttp server is no longer its own module, since the examples module is already on go 1.24, so we don't need to special case it. Co-Authored-By: Claude Opus 4.6 <[email protected]>
mromaszewicz
force-pushed
the
cleanup-petstore-examples
branch
from
a626351 to
f9e23b1
Compare
jamietanna
left a comment
jamietanna
left a comment
There was a problem hiding this comment.
Although I see the benefit I do wonder if we should embrace the duplication - I think the original intent of this was "look how you'd be able to implement the simple 'petstore' API, with each of these routers"
If we set up shared code, that defeats the point
Not necessarily saying 'no' here, but wondering if we should think if this is the right approach - or whether there's a simpler/different API surface that we want to introduce an example for, where we can implement it once-per-Router?
|
I'm of two minds on this one. I did this because this started out as one single example with Echo, my initial HTTP router. Now, all these implementations are copy/pasted from each other, and half of them didn't actually work properly (a generated client could not communicate with a generated server). So, yes, a single example is more convoluted now, however, it's much more obvious which parts are generic and which parts are router-specific. As-is in this PR, the client can communicate with all the back ends and adding another backend is much easier. I can drop this refactor and fix up the broken servers independently, but I was thinking of how to make this more thorough and better tested and I figured a shared "store" implementation and a test via the generatd client code were pretty good. |
2 participants
Extract server setup from main() into server/setup.go for each of the 9 variants (chi, gorilla, stdhttp, strict, gin, echo, echo-v5, fiber, iris). Each setup.go exports a factory function (NewServer, NewEchoServer, NewFiberApp, or NewIrisApp) that loads the swagger spec, creates the router, adds validation middleware, and registers handlers — returning the configured server/app and an error instead of calling os.Exit().
Replace per-variant petstore_test.go files with integration/main.go programs that start the server on a random port, exercise all CRUD operations via the shared testclient.Run(), and shut down cleanly. During this work we discovered that several server variants (gin, fiber, iris, and others) had server implementations that were not actually compatible with the generated HTTP client — the prior unit tests sidestepped this by testing against the handler directly with testutil rather than going through real HTTP. The shared test client exposed these gaps, which were fixed in earlier phases of this refactoring.
Each petstore.go is now a thin wrapper: parse flags, call the factory, set the address, and serve. The old NewGinPetServer, NewFiberPetServer, and NewIrisPetServer factory functions that lived in the main package (and called os.Exit on error) have been replaced by proper error- returning functions in the server package.
Also removes the echo/pkg_codegen_petstore_test.go codegen test and cleans up the now-unused golang.org/x/lint and testutil dependencies. README.md updated to document the new structure.