Skip to content

fix copyFilesystems raw-copy branch; cover with tests#6

Open
eriknordmark wants to merge 1 commit into
diskfs:mainfrom
eriknordmark:test-coverage-and-rawcopy-fix
Open

fix copyFilesystems raw-copy branch; cover with tests#6
eriknordmark wants to merge 1 commit into
diskfs:mainfrom
eriknordmark:test-coverage-and-rawcopy-fix

Conversation

@eriknordmark
Copy link
Copy Markdown

Summary

Two related changes:

Bug fix: raw-copy branch was unreachable

copyFilesystems detects "no recognized filesystem on source" via:

case err != nil && !errors.Is(err, &disk.UnknownFilesystemError{}):

But go-diskfs returns a fresh *UnknownFilesystemError with a populated partition field (disk.NewUnknownFilesystemError(partIndex)). errors.Is compares against the zero-valued sentinel &UnknownFilesystemError{} and never matches, so any source partition without a recognized filesystem (including a real squashfs we'd want to grow) falls into the rejection arm above instead of the raw-copy arm below.

Switched to an errors.As-based helper (isUnknownFilesystem) that ignores the per-instance field. This makes the raw-copy / squashfs path actually reachable, which is the path needed for partitions whose source is squashfs (a common shape: rootfs images grown to make room for larger replacements).

Test coverage for previously-uncovered paths

Four additions:

  • TestSwapPartitions — confirms swapPartitions round-trips Name / Type-GUID / PartUUID / Attributes between the original slot and the alt-labeled new slot, and preserves geometry. Catches inversion-on-double-call regressions.
  • TestShrinkFilesystems — implements the previously-empty stub. Mocks execResize2fs and covers: skip when current ≤ target, invocation when shrinking, error propagation, and rejection of non-ext4 sources.
  • TestCopyFilesystemsRawCopy — exercises the freshly-reachable raw-copy branch via an unrecognized-filesystem source. Uses equal-sized partitions to avoid an unrelated go-diskfs verification bug (sync.verifyBlockCopy rejects target-larger-than-source; filed at sync.verifyBlockCopy fails when target partition is larger than source go-diskfs#403).
  • TestCreatePartitions update — corrects the expectation: createPartitions intentionally assigns the alternate label (<orig>_resized2) to the new slot; a later swapPartitions step is what restores the original name. The previous expectation asserted the post-swap state but was running before swap. Verified passes against both unmodified upstream go-diskfs and a local patched build.

Test results

ok  	github.com/diskfs/partitionresizer  ...
ok  	github.com/diskfs/partitionresizer/cmd/resizer

(Pre-existing TestRun failure remains; that's blocked on an unrelated go-diskfs ext4 issue — diskfs/go-diskfs#402 — and not in scope here.)

Notes for review

  • Per-phase functions (shrinkFilesystems, createPartitions, copyFilesystems, swapPartitions, removePartitions) remain unexported. Callers that want explicit per-phase orchestration would need them exported; that's a separate API question, not part of this PR. Discussed briefly in Add idempotency #2 (comment).

Signed-off-by: eriknordmark erik@zededa.com

@deitch
Copy link
Copy Markdown
Contributor

deitch commented May 28, 2026

Looks like linting errors.

Two related changes:

The raw-data-copy branch in copyFilesystems used
`errors.Is(err, &disk.UnknownFilesystemError{})` to detect "no
recognized filesystem on source partition", but go-diskfs returns a
fresh `*UnknownFilesystemError` with a populated partition field via
`NewUnknownFilesystemError(partIndex)`. The zero-instance comparison
through errors.Is never matches, so the branch never fires and any
source partition without a known filesystem (including a real
squashfs we want to grow) falls into the rejection arm. Replace with
an errors.As-based helper.

Add four tests for paths that were previously uncovered:

- TestSwapPartitions: confirms swapPartitions round-trips
  Name/Type-GUID/PartUUID/Attributes between the original slot and
  the alt-labeled new slot, with geometry preserved.
- TestShrinkFilesystems: implements the previously-empty stub.
  Mocks execResize2fs and covers the skip-when-no-shrink-needed
  paths, the invocation when shrinking, error propagation, and
  rejection of non-ext4 sources.
- TestCopyFilesystemsRawCopy: exercises the freshly-reachable
  raw-copy branch via an unrecognized-filesystem source.
- Update TestCreatePartitions to expect the alternate label
  (createPartitions assigns altName; the swap step is what restores
  the original name on the new slot).

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@eriknordmark eriknordmark force-pushed the test-coverage-and-rawcopy-fix branch from 4312d66 to 7cce065 Compare May 29, 2026 16:20
@eriknordmark eriknordmark requested a review from deitch May 29, 2026 16:47
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.

2 participants