refactor(webui): Migrate example tests to new Fastify architecture. - #1099
Conversation
WalkthroughThe changes remove the registration of the example API route from the main Fastify application, refactor the example route plugin for improved type safety, and update the associated test to directly register the route. No other routes or plugin configurations are affected. Changes
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}Instructions used from: Sources: 🧠 Learnings (3)📓 Common learningscomponents/webui/server/src/test/example.test.ts (3)components/webui/server/src/fastify-v2/routes/api/example/index.ts (2)🔇 Additional comments (6)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
hoophalab
left a comment
There was a problem hiding this comment.
lgtm. No validation is needed because currently the test code is a place holder and doesn't test anything.
| let resp = await server.inject({ | ||
| method: "GET", |
There was a problem hiding this comment.
github cannot comment on unchanged lines, but the url below this line should be "/api/example/get/Alice" right?
There was a problem hiding this comment.
annoyingly no because the route is imported directly, i believe fastify only takes the directory structure if you use the auto import method we are using in app.ts.
So this is correct
|
fyi u can test with npm run test. u should see all pass |
junhaoliao
left a comment
There was a problem hiding this comment.
lgtm. deferring to @hoophalab 's review
…-scope#1099) Co-authored-by: Marco <david.marcovitch@yscope.com>
Description
PR moves example tests into new fastify architecture. This is a chain of PRs which started with #1027
I removed building the app from the test, since we are only testing the route.
I left some unused test helpers since they are new(after refactor), and may be useful for future integration tests.
Checklist
breaking change.
Validation performed
Tests pass
Summary by CodeRabbit