Create local example form for example tests#308
Conversation
Reviewer's GuideReplaces the external httpbin form dependency in the getting-started example with a local HTML pizza order form that mimics httpbin’s form echo behavior, ensuring example tests run without network access and adds a uv.lock file. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The new
form_path/form_urllogic assumes the HTML file is always present on disk; consider adding a simple existence check or a clearer error message ifpizza_form.htmlis missing to make failures easier to diagnose. - The new
uv.lockfile at the repo root looks unrelated to the example change; double-check whether it is meant to be versioned here or should be excluded from this PR.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `form_path`/`form_url` logic assumes the HTML file is always present on disk; consider adding a simple existence check or a clearer error message if `pizza_form.html` is missing to make failures easier to diagnose.
- The new `uv.lock` file at the repo root looks unrelated to the example change; double-check whether it is meant to be versioned here or should be excluded from this PR.
## Individual Comments
### Comment 1
<location path="docs/examples/getting-started/pizza_form.html" line_range="8" />
<code_context>
+ <title>Pizza Order Form</title>
+</head>
+<body id="page-body">
+ <form id="pizza-form" action="javascript:void(0);" method="post">
+ <label for="custname">Customer name:</label>
+ <input type="text" id="custname" name="custname"><br>
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Avoid using a `javascript:` URL in the `action` attribute and rely on the submit handler instead.
Since the `submit` handler already calls `preventDefault()`, this `action` is unnecessary, and `javascript:` URLs are generally discouraged for security and readability. Consider omitting `action` or using `"#"`/a real endpoint instead.
```suggestion
<form id="pizza-form" method="post">
```
</issue_to_address>
### Comment 2
<location path="docs/examples/getting-started/pizza_form.html" line_range="59-60" />
<code_context>
+ }
+ });
+
+ // Replace the entire page body with the JSON response (mirrors httpbin behaviour)
+ document.body.innerHTML = JSON.stringify(data);
+ });
+ </script>
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Use textContent (and optionally a <pre>) instead of innerHTML for rendering JSON.
Using `innerHTML` here means any `<` or `&` in the JSON could be interpreted as HTML, which is unsafe and not how JSON is typically rendered. To keep it as plain text while preserving formatting, you could create a `<pre>` element and set its `textContent`, e.g.:
```js
document.body.innerHTML = '<pre></pre>';
document.querySelector('pre').textContent = JSON.stringify(data, null, 2);
```
This avoids HTML parsing while still showing the JSON nicely formatted.
```suggestion
// Replace the entire page body with the JSON response (mirrors httpbin behaviour),
// rendering it as plain text inside a <pre> to avoid HTML parsing.
document.body.innerHTML = '<pre></pre>';
document.querySelector('pre').textContent = JSON.stringify(data, null, 2);
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR removes the getting-started example’s dependency on an external website by switching the demo flow to a local HTML form, improving reliability for running the example and for CI environments without network access.
Changes:
- Added a local
pizza_form.htmlpage that mimics the prior form/response flow. - Updated
first_script.pyto navigate to the local HTML file via afile://URI.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| docs/examples/getting-started/pizza_form.html | Adds a local form page and client-side “response” rendering to emulate the prior external form endpoint. |
| docs/examples/getting-started/first_script.py | Switches navigation from the external URL to a local pizza_form.html file resolved at runtime. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #308 +/- ##
=======================================
Coverage 92.72% 92.72%
=======================================
Files 19 19
Lines 2750 2750
=======================================
Hits 2550 2550
Misses 200 200
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
d43e831 to
02f7a1b
Compare
the external depedency isn't loading anymore Co-authored-by: Claude <noreply@anthropic.com>
02f7a1b to
e6689c1
Compare
the external depedency isn't loading anymore, tests on main branch are failing
Summary by Sourcery
Replace the getting-started example form flow to use a local HTML page instead of an external website, removing the dependency on external network resources for example execution and tests.
Enhancements:
Chores: