Skip to content

fix: correct TOML deserialization test for SerializableBranch empty/whitespace rejection#37

Merged
bashandbone merged 2 commits intofeat/test-expansionfrom
copilot/sub-pr-33
Mar 19, 2026
Merged

fix: correct TOML deserialization test for SerializableBranch empty/whitespace rejection#37
bashandbone merged 2 commits intofeat/test-expansionfrom
copilot/sub-pr-33

Conversation

Copy link
Contributor

Copilot AI commented Mar 19, 2026

test_branch_deserialize_from_toml_rejects_empty_and_whitespace was failing because it attempted to deserialize directly into SerializableBranch from a TOML key-value string — but TOML's top-level must be a table, so the deserializer received a map instead of a string and failed before reaching any branch validation logic.

Fix

  • Wrap SerializableBranch in a helper struct for the test, matching how it actually appears in real config files:
#[derive(Debug, serde::Deserialize)]
struct Helper {
    branch: SerializableBranch,
}

let res: Result<Helper, toml::de::Error> = toml::from_str(r#"branch = """#);
// Now correctly reaches SerializableBranch::deserialize and emits "invalid branch value"

Previously, toml::from_str::<SerializableBranch>("branch = \"\"") produced invalid type: map, expected a string instead of the expected invalid branch value error.


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…rly isolate SerializableBranch

Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix serialization/deserialization bug and expand testing fix: correct TOML deserialization test for SerializableBranch empty/whitespace rejection Mar 19, 2026
Copilot AI requested a review from bashandbone March 19, 2026 15:29
@bashandbone bashandbone marked this pull request as ready for review March 19, 2026 16:19
Copilot AI review requested due to automatic review settings March 19, 2026 16:19
@bashandbone bashandbone merged commit 58d7a48 into feat/test-expansion Mar 19, 2026
1 check passed
@bashandbone bashandbone deleted the copilot/sub-pr-33 branch March 19, 2026 16:20
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates a TOML deserialization test in src/options.rs to reflect how SerializableBranch is represented in real config files (as a field inside a TOML table), so the test reaches SerializableBranch’s validation logic instead of failing at the TOML top-level type.

Changes:

  • Wrap SerializableBranch in a local helper struct for TOML deserialization in the test.
  • Adjust the test inputs for empty and whitespace-only branch values (note: current raw strings are malformed and will still fail TOML parsing before validation).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 726 to 728
// Empty string should be rejected
let res_empty: Result<SerializableBranch, toml::de::Error> =
toml::from_str("branch = \"\"");
let res_empty: Result<Helper, toml::de::Error> = toml::from_str(r#"branch = """#);
assert!(res_empty.is_err(), "expected error for empty branch value");
// Whitespace-only string should be rejected
let res_ws: Result<SerializableBranch, toml::de::Error> =
toml::from_str("branch = \" \"");
let res_ws: Result<Helper, toml::de::Error> = toml::from_str(r#"branch = " ""#);
bashandbone added a commit that referenced this pull request Mar 19, 2026
…in core areas. (#33)

* feat: Add schema; delete old CLAUDE.md for regeneration.

* fix: Fixed an issue with toml deserialization due to mismatched expectations between serialize and deserialize forms. Caught by some of the new expanded tests.

* Apply suggestions from code review

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix: correct fetchRecurseSubmodules key lookup in from_gitmodules (#34)

* Initial plan

* fix: use correct fetchRecurseSubmodules key in from_gitmodules with fetchRecurse fallback

Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>

* fix: SPDX headers in CLAUDE.md; delegate SerializableBranch deserialization to from_gitmodules (#36)

* Initial plan

* fix: add SPDX headers to CLAUDE.md; delegate SerializableBranch deserialization to from_gitmodules

Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Relocate branch deserialization test to correct module

Moved the test for branch deserialization from TOML to the appropriate test module.

* fix: correct TOML deserialization test for SerializableBranch empty/whitespace rejection (#37)

* Initial plan

* fix: use wrapper struct for branch TOML deserialization test to properly isolate SerializableBranch

Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants