-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add more test cases for anomalous conditions #10
Conversation
Some conditions must cause errors.
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #10 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 9 9
Lines 132 142 +10
Branches 8 15 +7
=========================================
+ Hits 132 142 +10 ☔ View full report in Codecov by Sentry. |
WalkthroughThis pull request introduces enhancements to the test suites for the Changes
Possibly related PRs
Warning Tool Failures:Tool Failure Count:📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🔇 Additional comments (5)src/components/Bio/Bio.test.ts (1)
The new assertion verifies that the Bio component includes proper alt text for the profile picture, which is essential for accessibility. src/components/Homepage/Homepage.test.ts (2)
The explicit import of Props type enhances type checking and maintains better type safety throughout the tests.
Enhance error validation specificity. While testing for error throwing is good, it would be more robust to verify the specific type or message of the error. Consider modifying the test to check for a specific error: await expect(
container.renderToString(Homepage, {
props: { url: "not-valid-url" } satisfies Props,
}),
-).rejects.toThrow();
+).rejects.toThrow(/Invalid URL/); // Or the expected error message Also, consider adding more invalid URL test cases:
src/libs/contact.ts (2)
The
The function throws an error for unsupported services, which is good practice. Ensure that the Run the following script to list all services defined in ✅ Verification successfulThe previous command failed due to regex syntax. Let's try a different approach to locate and analyze the services definition in the codebase. We'll first find files containing "services" and then examine their contents for the relevant definitions. Services object is properly defined with common social media platforms The
Each service has the required properties ( 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: List all services defined in the services object.
rg 'services = {' ../types/services.ts -A 20
Length of output: 134 Script: #!/bin/bash
# First, let's find the correct file that defines the services
fd services
# Then, let's look for any object declarations containing services
rg "services\s*=\s*[\{]" -A 20
# As a backup, let's also search for service-related type definitions
ast-grep --pattern 'type $_ = {
$$$
}'
Length of output: 1099 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (8)
src/components/Bio/Bio.test.ts (1)
Line range hint
8-17
: Consider adding test cases for anomalous conditions.While the current tests cover the happy path, considering the PR's objective to test anomalous conditions, consider adding test cases for:
- Missing or empty props
- Invalid icon URLs
- Special characters in name/description
Here's a suggested addition:
test("handles missing props gracefully", async () => { const container: AstroContainer = await AstroContainer.create(); const invalidProps = { name: "", description: undefined, icon: null }; await expect(async () => { await container.renderToString(Bio, { props: invalidProps }); }).rejects.toThrow(); });src/components/Homepage/Homepage.test.ts (2)
9-27
: Consider enhancing assertion messages for better test failure debugging.The test structure and data-driven approach are excellent. However, the assertions could be more descriptive.
Consider adding custom messages to assertions:
expect(result).toContain( `> ${encodeURI(url.hostname)}${ url.pathname === "/" ? "" : url.pathname } </span>`, -); +), `URL parts not properly rendered for ${name}`; expect(result).toContain('<use href="#ai:'), + `Icon reference missing for ${name}`; expect(result).toContain("aria-label"), + `Accessibility attribute missing for ${name}`;
Line range hint
1-37
: Consider extracting test data for invalid cases.The test structure is well-organized, but consider extracting invalid URL test cases into a separate story file (e.g.,
invalidStories.ts
) to maintain consistency with the data-driven approach used for valid URLs. This would make it easier to add more error cases in the future.src/components/Contact/Contact.test.ts (3)
8-22
: LGTM! Consider adding documentation for test cases.The test structure and assertions are well-organized, covering multiple aspects of the component rendering. The new assertions for href, icon, and aria-label improve the test coverage significantly.
Consider adding a comment block describing the test cases from story.ts for better readability and maintainability. For example:
// Test cases from story.ts cover: // - Mastodon handles (@user@instance) // - GitHub usernames // - etc.
31-31
: Remove unused parameter in for...of loop.The underscore parameter is unused and can be removed.
- for (const [_, props] of Object.entries(invalidProps)) { + for (const { service, id } of invalidProps) {
25-29
: Enhance error case coverage and validation.While the current test cases cover basic invalid scenarios, consider:
- Adding more edge cases:
- Empty strings
- Special characters
- Very long IDs
- Validating specific error messages to ensure proper error handling
Example implementation:
const invalidProps: { service: string; id: string }[] = [ { service: "Mastodon", id: "invalid-id" }, { service: "Misskey", id: "invalid-id" }, { service: "invalid-service", id: "id" }, + { service: "Mastodon", id: "" }, + { service: "", id: "valid-id" }, + { service: "Mastodon", id: "a".repeat(256) }, + { service: "Mastodon", id: "test@test@test" }, ]; - test(props.service, async () => { + test(`${props.service} with ${props.id}`, async () => { const container: AstroContainer = await AstroContainer.create(); await expect( container.renderToString(Contact, { props }), - ).rejects.toThrow(); + ).rejects.toThrow(/Invalid (service|ID) format/); });Also applies to: 32-38
src/libs/contact.ts (2)
14-19
: Optimize by reusing the splitid
parts.The
id
is split usingid.split("@")
in both the validation check and URL construction. To avoid redundant computation, consider storing the split parts once and reusing them.Apply this diff to store and reuse the
parts
array:if (isFediverse(service)) { const parts: string[] = id.split("@"); if (parts.length !== 3 || parts[0] !== "") { throw new Error(`Invalid ${service} ID format`); } + const instance = parts[2]; + const username = parts[1]; } const url: string = isFediverse(service) - ? `https://${id.split("@")[2]}/${id}` + ? `https://${instance}/@${username}` : specified.url + id;
16-18
: Enhance error message for clarity.When the
id
format is invalid, providing the expected format can help users correct their input more easily.Consider updating the error message as follows:
if (parts.length !== 3 || parts[0] !== "") { - throw new Error(`Invalid ${service} ID format`); + throw new Error(`Invalid ${service} ID format. Expected format: "@username@instance"`); }
🛑 Comments failed to post (1)
src/libs/contact.ts (1)
21-23:
⚠️ Potential issueValidate and sanitize user input to prevent security risks.
Directly using user-provided
id
components in URL construction can expose the application to injection attacks or malformed URLs. It's important to validate and sanitize these inputs.Consider adding validation to ensure
username
andinstance
contain only acceptable characters (e.g., alphanumerics and standard symbols) before using them in the URL.
<!-- Release notes generated using configuration in .github/release.yml at main --> * Add bio component by @5ouma in #5 * Add homepage component by @5ouma in #7 * Add contact component by @5ouma in #9 * Add index page and default layout template by @5ouma in #13 * Astro requires in-file CSS for scoping by @5ouma in #6 * Add README and license by @5ouma in #11 * Exclude Astro and Astrobook related by @5ouma in #8 * Add more test cases for anomalous conditions by @5ouma in #10 * Allow commenting on commits by @5ouma in #3 * Deploy and analyze performance by @5ouma in #12 * Quote meta file variable by @5ouma in #14 * Change the environment variable for repository name by @5ouma in #15 * Don't treat the input as JSON by @5ouma in #16 * chore(deps): pin koki-develop/bun-diff-action action to 22bcd25 by @renovate in #4 * @5ouma made their first contribution in #1 * @renovate made their first contribution in #4 * @github-actions made their first contribution in #2 **Full Changelog**: https://github.com/5ouma/mobicard/commits/v0.1.0
<!-- Release notes generated using configuration in .github/release.yml at main --> * Add bio component by @5ouma in #5 * Add homepage component by @5ouma in #7 * Add contact component by @5ouma in #9 * Add index page and default layout template by @5ouma in #13 * Astro requires in-file CSS for scoping by @5ouma in #6 * Add README and license by @5ouma in #11 * Exclude Astro and Astrobook related by @5ouma in #8 * Add more test cases for anomalous conditions by @5ouma in #10 * Allow commenting on commits by @5ouma in #3 * Deploy and analyze performance by @5ouma in #12 * Quote meta file variable by @5ouma in #14 * Change the environment variable for repository name by @5ouma in #15 * Don't treat the input as JSON by @5ouma in #16 * chore(deps): pin koki-develop/bun-diff-action action to 22bcd25 by @renovate in #4 * @5ouma made their first contribution in #1 * @renovate made their first contribution in #4 * @github-actions made their first contribution in #2 **Full Changelog**: https://github.com/5ouma/mobicard/commits/v0.1.0
<!-- Release notes generated using configuration in .github/release.yml at main --> * Add bio component by @5ouma in #5 * Add homepage component by @5ouma in #7 * Add contact component by @5ouma in #9 * Add index page and default layout template by @5ouma in #13 * Astro requires in-file CSS for scoping by @5ouma in #6 * Add README and license by @5ouma in #11 * Exclude Astro and Astrobook related by @5ouma in #8 * Add more test cases for anomalous conditions by @5ouma in #10 * Allow commenting on commits by @5ouma in #3 * Deploy and analyze performance by @5ouma in #12 * Quote meta file variable by @5ouma in #14 * Change the environment variable for repository name by @5ouma in #15 * Don't treat the input as JSON by @5ouma in #16 * chore(deps): pin koki-develop/bun-diff-action action to 22bcd25 by @renovate in #4 * @5ouma made their first contribution in #1 * @renovate made their first contribution in #4 * @github-actions made their first contribution in #2 **Full Changelog**: https://github.com/5ouma/mobicard/commits/v0.1.0
close #
✏️ Description
Some conditions must cause errors.
🔄 Type of the Change