Repository navigation
feat: add hello world example and rework http client sample - #122
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR reorganizes the examples into numbered hello-world, HTTP client, and HTTP service projects. It adds package metadata and fixtures, updates example paths, removes the former HTTP client, and expands E2E output validation with number, package, and timestamp placeholders. ChangesNumbered example projects
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Example as HTTP client example
participant Registry as Ballerina package registry
participant Validator as E2E output validator
Example->>Registry: Request AWS package data
Registry-->>Example: Return package search JSON
Example->>Example: Filter and rank packages
Example-->>Validator: Print package results
Validator->>Validator: Match placeholder-aware fixture pattern
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
|
@TharmiganK Could you also update the example output fixtures by running: |
|
@TharmiganK Since the packages and pull counts are dynamic, we cannot maintain a fixed output fixture for this example, right? I had to normalize the Ballerina logs for the HTTP service example as well. playground/e2e/helpers/example-output.ts Lines 19 to 21 in 9282fba |
Yes, will follow this for the client test as well |
933f0fe to
4960e79
Compare
snelusha
left a comment
There was a problem hiding this comment.
LGTM, just a small nitpick.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
e2e/fixtures/examples/01-http-client.txt (1)
1-6: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd semantic assertions for the dynamic rows.
The placeholders validate only output shape.
<number>accepts0, and<package>accepts any package. A regression can remove thepulls > 100filter, change descending order, or emit duplicate rows without failing this fixture. Keep the placeholders, but add assertions for five unique rows with pull counts greater than100in non-increasing order.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/fixtures/examples/01-http-client.txt` around lines 1 - 6, Strengthen the assertions in the “Top 5 popular packages” fixture while retaining the existing <package> and <number> placeholders: require five distinct package rows, each with pulls greater than 100, and validate that pull counts are in non-increasing order.e2e/helpers/example-output.ts (1)
62-64: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winTreat only missing fixtures as absent.
The catch-all handler hides permission, path, and I/O errors from
readFile. CatchENOENTonly and rethrow other errors so fixture failures remain visible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/helpers/example-output.ts` around lines 62 - 64, Update the loadExampleOutput call in the existingOutput initialization to catch only errors with code ENOENT and return null for those missing fixtures. Rethrow all other errors, including permission, path, and I/O failures, so loadExampleOutput failures remain visible.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/helpers/example-output.ts`:
- Around line 58-71: Update the example-output update flow after the
existingOutput pattern check to canonicalize volatile time= values before
writing the fixture. Ensure the fallback handled by the surrounding
output-generation function passes the normalized output to writeFile while
preserving all stable text unchanged.
---
Nitpick comments:
In `@e2e/fixtures/examples/01-http-client.txt`:
- Around line 1-6: Strengthen the assertions in the “Top 5 popular packages”
fixture while retaining the existing <package> and <number> placeholders:
require five distinct package rows, each with pulls greater than 100, and
validate that pull counts are in non-increasing order.
In `@e2e/helpers/example-output.ts`:
- Around line 62-64: Update the loadExampleOutput call in the existingOutput
initialization to catch only errors with code ENOENT and return null for those
missing fixtures. Rethrow all other errors, including permission, path, and I/O
failures, so loadExampleOutput failures remain visible.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 31aec7c2-be89-4bfe-b84e-0bb8ee719220
📒 Files selected for processing (3)
e2e/fixtures/examples/01-http-client.txte2e/helpers/example-output.tsexamples/01-http-client/main.bal
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/01-http-client/main.bal
TharmiganK
left a comment
There was a problem hiding this comment.
Claude PR Review Findings
The HTTP client example now queries Ballerina Central, whose package ranking, versions and pull counts all change upstream, so an exact snapshot cannot hold. Support <package> and <number> placeholders in example fixtures and use them for the HTTP client example. Placeholders are scoped to the fixtures that need them so every other example keeps exact-match assertions, and the fixture update mode leaves a fixture alone while its placeholders still cover the output.
Timestamps varied for the same reason packages and pull counts do, so drop the separate regex-based normalization step and add a <timestamp> placeholder to the same mechanism, matching how fixtures already handle live-service output.
Regenerating a fixture wrote the pane's raw time= value straight to disk, so the very next normal run failed since that literal timestamp never recurs. Canonicalize back to the <timestamp> placeholder only at the write site, leaving the match-against-existing-fixture check untouched.
The registry API paginates at 15 packages per request by default, so the top-5-by-pulls ranking was computed over a possibly incomplete result set for broader queries. Request a larger limit in the single call instead. The header also always said "Top 5" regardless of how many packages actually cleared the pull-count threshold; print the real count instead. Also refuse to silently regenerate a fixture that relies on <package>/<number> placeholders when it no longer matches — those values can't be canonicalized back out of raw output, so overwriting would re-bake today's literals and break again on the next run.
Renumber http-client to 02 and http-service to 03 to make room for a minimal hello-world example as 01, and point the playground's default opened file at it so it's what loads first.
5922cab to
3a36129
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/fixtures/examples/02-http-client.txt`:
- Around line 1-6: Update the popular-packages fixture so it accepts any number
of matching package rows up to five rather than requiring exactly five rows.
Preserve the existing query and pull-count output format while making the row
section variable-length, or replace it with deterministic data that guarantees
five matches.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 24840596-c3da-403f-82f8-0f5e57cb6ebd
📒 Files selected for processing (14)
apps/web/src/components/file-route-sync.tsxe2e/fixtures/examples/01-hello-world.txte2e/fixtures/examples/01-http-client.txte2e/fixtures/examples/02-http-client.txte2e/fixtures/examples/03-http-service.txte2e/helpers/example-output.tse2e/tests/examples.spec.tsexamples/01-hello-world/Ballerina.tomlexamples/01-hello-world/main.balexamples/01-http-client/main.balexamples/02-http-client/Ballerina.tomlexamples/02-http-client/main.balexamples/03-http-service/Ballerina.tomlexamples/03-http-service/main.bal
💤 Files with no reviewable changes (2)
- e2e/fixtures/examples/01-http-client.txt
- examples/01-http-client/main.bal
854632c to
3a36129
Compare
Purpose
Changes
<package>,<number>, and<timestamp>placeholders let a fixture match live/volatile output while everything else around them still matches exactly. Regenerating a fixture (UPDATE_EXAMPLE_OUTPUTS=1) now refuses to silently overwrite one that relies on<package>/<number>placeholders when it no longer matches, since those values can't be canonicalized back out of raw output.01-hello-worldand set as the playground's default file; the client and service examples are renumbered to02and03.Summary by CodeRabbit