Skip to content

fix(tests): correct inverted assertions for read-only snapshot property editing - #18501

Draft
mvanhorn wants to merge 1 commit into
canonical:mainfrom
mvanhorn:fix/18398-invert-snapshot-test-assertions
Draft

fix(tests): correct inverted assertions for read-only snapshot property editing#18501
mvanhorn wants to merge 1 commit into
canonical:mainfrom
mvanhorn:fix/18398-invert-snapshot-test-assertions

Conversation

@mvanhorn

Copy link
Copy Markdown

Checklist

Summary

  • test/suites/snapshots.sh and test/suites/storage_snapshots.sh both contain inverted test assertions that cause the tests to pass when editing read-only snapshot properties silently succeeds (the broken behavior) and fail when the API correctly rejects such edits.
  • The pipeline ! cmd 2>&1 | grep -xF "$MSG" || false negates grep's exit code, so the test passes when the error message is absent — exactly the opposite of the intended assertion.
  • Replaces the inverted pipeline with the pattern identified by @simondeziel in issue Editing read-only properties is silently ignored #18398: capture output into a variable, assert the command exits non-zero, then assert the expected error string is present.

Why this matters

The inverted assertions mean the regression introduced in #17070 (addressing #14423) was never caught by the test suite. A member maintainer (@simondeziel) diagnosed the flaw and provided the corrected pattern in the issue thread. This PR applies that pattern to both affected test files.

Testing

These are shell test files with no production code changes. Bash syntax has been verified (bash -n). The corrected assertions will:

  • Fail when lxc config edit foo/snap0 with a modified read-only field silently returns exit 0 (regression guard active)
  • Pass when the API correctly returns a non-zero exit code with the expected "Only 'expires_at' field(s) can be modified" error message
  • Behave identically in the storage_snapshots.sh parallel case for storage volume snapshots
  • Leave the positive expires_at-editable test immediately following the loop unaffected

Fixes #18398

…ty editing

The pipeline `! cmd 2>&1 | grep -xF "$MSG" || false` in snapshots.sh
and storage_snapshots.sh negates grep's exit code, causing tests to pass
when the error message is absent (the broken case) and fail when it is
present (the correct case). Replace with a pattern that captures output
into a variable, asserts the command exits non-zero, then asserts the
expected error message is present in the output.

Fixes canonical#18398

Signed-off-by: Matt Van Horn <mvanhorn@gmail.com>
@simondeziel

Copy link
Copy Markdown
Member

@mvanhorn thanks for showing interest on working on this! I see that you've provided test fixes but AFAIK, the main issue lies in the Go code.

@tomponline
tomponline requested a review from tugbataluy June 19, 2026 15:24
@simondeziel
simondeziel marked this pull request as draft June 19, 2026 15:51
@simondeziel

Copy link
Copy Markdown
Member

I've put it to draft until the Go code change lands.

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.

Editing read-only properties is silently ignored

2 participants