Skip to content

Conversation

@Shane32
Copy link
Owner

@Shane32 Shane32 commented Oct 7, 2025

Summary by CodeRabbit

  • New Features
    • None.
  • Bug Fixes
    • More specific error is now reported when embedded PNG data in SVG output cannot be found, improving diagnostics.
  • Chores
    • Broadened compatibility by simplifying conditional compilation for System.Drawing across .NET versions.
  • Tests
    • Updated tests to align with refined error handling and compilation conditions.

@Shane32 Shane32 self-assigned this Oct 7, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 7, 2025

📝 Walkthrough

Walkthrough

Updates a test file to broaden conditional compilation from “SYSTEM_DRAWING && !NET5_0_OR_GREATER” to “SYSTEM_DRAWING” and replace generic Exception throws with InvalidOperationException in three SVG PNG-embedding tests. No public API changes.

Changes

Cohort / File(s) Summary
Tests: SVG QRCode renderer
QRCoderTests/SvgQRCodeRendererTests.cs
Simplified preprocessor guard to #if SYSTEM_DRAWING. Replaced three generic Exception throws with InvalidOperationException when embedded PNG data in SVG is not found in specific tests.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • gfoidl

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “Enable SVG tests” directly conveys the principal change of this pull request, namely activating the SVG test suite by adjusting compilation directives. It is concise, clear, and specific enough for a teammate reviewing the commit history to understand the primary purpose without extraneous detail.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch enable_svg_tests

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f1ddb37 and 1d36bb1.

📒 Files selected for processing (1)
  • QRCoderTests/SvgQRCodeRendererTests.cs (4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Test .NET 6.0 Windows
  • GitHub Check: Test .NET 5.0 Windows
  • GitHub Check: Test .NET Core 3.1
  • GitHub Check: Test .NET Framework 4.6.2
  • GitHub Check: Test .NET Core 2.1
  • GitHub Check: additional-tests
🔇 Additional comments (2)
QRCoderTests/SvgQRCodeRendererTests.cs (2)

86-86: LGTM: Improved exception specificity.

Replacing generic Exception with InvalidOperationException is more semantically appropriate when the embedded PNG data cannot be found in the SVG output, as it clearly indicates an unexpected operational state.

Also applies to: 114-114, 142-142


65-65: Verify PNG logo tests on .NET 5.0+

Removal of the !NET5_0_OR_GREATER directive re-enables these three tests; confirm they pass and produce valid SVG outputs under .NET 5.0 and later.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Shane32 Shane32 merged commit 696bb39 into master Oct 7, 2025
8 checks passed
@Shane32 Shane32 deleted the enable_svg_tests branch October 7, 2025 22:52
@Shane32 Shane32 added the tests Changes to tests with no project code changes label Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests Changes to tests with no project code changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants