Fix: Serialization/Deserialization bug; significantly expand testing in core areas. by bashandbone · Pull Request #33 · bashandbone/submod

bashandbone · 2026-03-19T13:03:12Z

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.

…tations between serialize and deserialize forms. Caught by some of the new expanded tests.

codecov · 2026-03-19T13:07:00Z

Codecov Report

❌ Patch coverage is 95.23810% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/options.rs	94.44%	1 Missing ⚠️

Files with missing lines	Coverage Δ
src/config.rs	`80.19% <100.00%> (+0.01%)`	⬆️
src/options.rs	`94.65% <94.44%> (-1.20%)`	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

This PR fixes a TOML serialization/deserialization mismatch for branch configuration (ensuring serialized form matches the deserializer’s expectations), adds a JSON schema update, and substantially expands test coverage across core config/options/shell and CLI workflows.

Changes:

Fix SerializableBranch serialization to emit the git/gitmodules-compatible string form.
Add/expand unit + integration tests for config/options behavior and CLI command contracts.
Update docs/supporting files (schema, CLAUDE.md) and adjust dependencies.

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/sparse_checkout_tests.rs	Reformat CLI argument arrays for readability/consistency.
tests/performance_tests.rs	Reformat CLI argument arrays in performance loop.
tests/integration_tests.rs	Reformat argument arrays and assertion formatting.
tests/error_handling_tests.rs	Reformat argument arrays and minor formatting cleanup.
tests/command_contract_tests.rs	Reformat argument arrays/assertions; minor readability tweaks.
src/utilities.rs	Add extensive unit tests for utility helpers.
src/shells.rs	Add extensive unit tests for shell parsing/conversions/completions.
src/options.rs	Fix `SerializableBranch` serialization; add comprehensive tests for option wrappers.
src/main.rs	Formatting-only changes to iterator chaining for readability.
src/git_ops/simple_gix.rs	Import formatting only.
src/git_ops/mod.rs	Formatting-only changes in `reopen` and cleanup command args.
src/git_ops/gix_ops.rs	Import formatting only.
src/git_manager.rs	Formatting-only changes; preserves behavior.
src/config.rs	Add a large unit test module covering config and submodule entry behavior.
src/commands.rs	Attribute formatting only for clap args/help strings.
schemas/v1.0.0/submod_config_v1.0.0.json	Improve branch description guidance (aliases and refs/heads note).
CLAUDE.md	Regenerated contributor/agent guidance (commands, architecture, conventions).
Cargo.toml	Dependency cleanup and add explicit `toml` dependency.
Cargo.lock	Lockfile updated to reflect dependency changes.
.serena/project.yml	Add Serena project configuration.
.serena/.gitignore	Ignore Serena cache and local project override file.

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

schemas/v1.0.0/submod_config_v1.0.0.json

src/config.rs

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

bashandbone · 2026-03-19T13:16:40Z

@copilot tests now fail after applying your suggestions. Please resolve and ensure all tests pass:

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 143 filtered out; finished in 0.00s

stderr ───

thread 'config::tests::test_entry_from_gitmodules_full' (9126) panicked at src/config.rs:1311:9:
assertion `left == right` failed
  left: None
 right: Some(Always)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Cancelling due to test failure: 3 tests still running
PASS [ 0.007s] ( 38/394) submod config::tests::test_entry_from_gitmodules_invalid_values_silently_ignored
PASS [ 0.008s] ( 39/394) submod config::tests::test_entry_from_gitmodules_minimal
PASS [ 0.005s] ( 40/394) submod config::tests::test_entry_git_options
────────────
Summary [ 0.073s] 40/394 tests run: 39 passed, 1 failed, 0 skipped
FAIL [ 0.007s] ( 37/394) submod config::tests::test_entry_from_gitmodules_full
warning: 354/394 tests were not run due to test failure (run with --no-fail-fast to run all tests, or run with --max-fail)
error: test run failed
error: process didn't exit successfully: /home/runner/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/cargo nextest run --manifest-path /home/runner/work/submod/submod/Cargo.toml --target-dir /home/runner/work/submod/submod/target/llvm-cov-target --all-features --workspace (exit status: 100)
Error: Process completed with exit code 100.

Copilot · 2026-03-19T13:16:48Z

@bashandbone I've opened a new pull request, #34, to work on those changes. Once the pull request is ready, I'll request review from you.

* 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>

Copilot

Pull request overview

This PR fixes a config serialization/deserialization mismatch (notably around branch handling) and aligns config parsing with Git’s fetchRecurseSubmodules naming, while substantially expanding test coverage across core CLI/config/git behaviors.

Changes:

Fix TOML (de)serialization for SerializableBranch by implementing custom Serialize to match string-based Deserialize.
Update gitmodules parsing to prefer fetchRecurseSubmodules (with backward-compatible fallback).
Add/expand extensive unit + integration/contract tests; update schema/docs and adjust dependencies.

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/sparse_checkout_tests.rs	Formatting updates to test command invocations.
tests/performance_tests.rs	Formatting updates to test command invocations.
tests/integration_tests.rs	Formatting updates and clearer assertions in integration tests.
tests/error_handling_tests.rs	Formatting updates to test command invocations and assertions.
tests/command_contract_tests.rs	Formatting updates; keeps contract tests readable and consistent.
src/utilities.rs	Adds many unit tests for utility helpers and edge cases.
src/shells.rs	Adds broad unit tests for parsing/display/conversions and generator output.
src/options.rs	Fixes `SerializableBranch` serialization to string; adds comprehensive option tests.
src/main.rs	Formatting-only refactor for readability around collecting submodule names.
src/git_ops/simple_gix.rs	Import formatting only.
src/git_ops/mod.rs	Formatting only in reopen + cleanup fallback sections.
src/git_ops/gix_ops.rs	Import formatting only.
src/git_manager.rs	Mostly formatting; retains config writing/merging logic and option normalization.
src/config.rs	Fixes `fetchRecurseSubmodules` key parsing (with fallback) and adds extensive unit tests.
src/commands.rs	Formatting-only changes to clap arg attributes.
schemas/v1.0.0/submod_config_v1.0.0.json	Clarifies branch alias behavior and guidance in schema docs.
CLAUDE.md	Rewrites developer guidance and workflow docs (but removes SPDX header).
Cargo.toml	Adjusts gitoxide/gix deps/features and adds explicit `toml` dependency.
Cargo.lock	Lockfile updates reflecting dependency changes.
.serena/project.yml	Adds Serena project configuration file.
.serena/.gitignore	Adds ignores for Serena cache/local config.

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

CLAUDE.md

 # CLAUDE.md

 This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.



src/options.rs

 impl<'de> Deserialize<'de> for SerializableBranch {
    /// Deserialize from a plain string using the same logic as [`FromStr`].
    /// Accepts `"."`, `"current"`, `"current-in-super-project"`, `"superproject"`, or `"super"`
    /// as [`CurrentInSuperproject`](SerializableBranch::CurrentInSuperproject); all other strings
    /// become [`Name`](SerializableBranch::Name).
    fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
        let s = String::deserialize(deserializer)?;
-        SerializableBranch::from_str(&s).map_err(|_| {
-            serde::de::Error::custom(format!("invalid branch value: {s}"))
-        })
+        SerializableBranch::from_str(&s)
+            .map_err(|_| serde::de::Error::custom(format!("invalid branch value: {s}")))
    }


bashandbone · 2026-03-19T13:52:47Z

@copilot open a new pull request to apply changes based on the comments in this thread

Ensure files have both FileCopyright and LicenseIdentifier headers. Implement the suggestion to delegate to from_gitmodules

Copilot · 2026-03-19T13:52:54Z

@bashandbone I've opened a new pull request, #36, to work on those changes. Once the pull request is ready, I'll request review from you.

…zation 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>

Copilot

Pull request overview

Fixes configuration parsing inconsistencies by aligning SerializableBranch TOML deserialization with the existing gitmodules parsing logic, and corrects fetchRecurse(Submodules) key handling when building SubmoduleEntry from gitmodules entries. Also updates the v1.0.0 JSON schema docs for the branch alias spelling.

Changes:

Delegate SerializableBranch TOML deserialization to GitmodulesConvert::from_gitmodules and add a TOML deserialization test for empty/whitespace values.
Prefer fetchRecurseSubmodules (with fallback to fetchRecurse) when parsing gitmodules entries into SubmoduleEntry.
Update schema docs to reference current-in-super-project alias spelling.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
`src/options.rs`	Changes branch deserialization behavior and adds a unit test for invalid branch values.
`src/config.rs`	Fixes gitmodules key lookup for fetch-recurse and updates the corresponding test.
`schemas/v1.0.0/submod_config_v1.0.0.json`	Updates documented branch alias spelling in schema description.

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

src/options.rs

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

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

bashandbone · 2026-03-19T15:23:06Z

@copilot we have two failing tests that are testing behavior before our changes in this PR process. Please correct them:

    FAIL [   0.007s] (241/396) submod::bin/submod options::tests::test_branch_deserialize_from_toml_rejects_empty_and_whitespace

stdout ───

running 1 test
test options::tests::test_branch_deserialize_from_toml_rejects_empty_and_whitespace ... FAILED

failures:

failures:
    options::tests::test_branch_deserialize_from_toml_rejects_empty_and_whitespace

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 144 filtered out; finished in 0.00s

stderr ───

thread 'options::tests::test_branch_deserialize_from_toml_rejects_empty_and_whitespace' (11783) panicked at src/options.rs:724:9:
error for empty branch value should contain context, got: TOML parse error at line 1, column 1
  |
1 | branch = ""
  | ^^^^^^^^^^^
invalid type: map, expected a string

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Copilot · 2026-03-19T15:23:15Z

@bashandbone I've opened a new pull request, #37, to work on those changes. Once the pull request is ready, I'll request review from you.

…hitespace 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>

bashandbone added 2 commits March 19, 2026 07:59

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

57247d5

fix: Fixed an issue with toml deserialization due to mismatched expec…

b13a162

…tations between serialize and deserialize forms. Caught by some of the new expanded tests.

Copilot AI review requested due to automatic review settings March 19, 2026 13:03

Copilot AI reviewed Mar 19, 2026

View reviewed changes

schemas/v1.0.0/submod_config_v1.0.0.json Outdated Show resolved Hide resolved

src/config.rs Outdated Show resolved Hide resolved

Apply suggestions from code review

fe7c58e

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

Copilot AI mentioned this pull request Mar 19, 2026

fix: correct fetchRecurseSubmodules key lookup in from_gitmodules #34

Merged

bashandbone requested a review from Copilot March 19, 2026 13:43

Copilot AI reviewed Mar 19, 2026

View reviewed changes

Copilot AI mentioned this pull request Mar 19, 2026

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

Merged

Copilot AI and others added 2 commits March 19, 2026 10:27

Merge branch 'main' into feat/test-expansion

09f7ccf

bashandbone requested a review from Copilot March 19, 2026 14:30

Copilot AI reviewed Mar 19, 2026

View reviewed changes

src/options.rs Outdated Show resolved Hide resolved

src/options.rs Outdated Show resolved Hide resolved

src/options.rs Outdated Show resolved Hide resolved

src/options.rs Outdated Show resolved Hide resolved

bashandbone and others added 2 commits March 19, 2026 10:38

Apply suggestions from code review

b69eeef

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

Relocate branch deserialization test to correct module

1104c95

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

Copilot AI mentioned this pull request Mar 19, 2026

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

Merged

		# CLAUDE.md

		This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.

Conversation

bashandbone commented Mar 19, 2026

Uh oh!

codecov bot commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

bashandbone commented Mar 19, 2026

Uh oh!

Copilot AI commented Mar 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

bashandbone commented Mar 19, 2026

Uh oh!

Copilot AI commented Mar 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bashandbone commented Mar 19, 2026

Uh oh!

Copilot AI commented Mar 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Mar 19, 2026 •

edited

Loading