Skip to content

Use single quotes for readability in error assertions#17455

Merged
tomponline merged 2 commits intocanonical:mainfrom
kadinsayani:lxd-fix-17125-follow-up
Jan 22, 2026
Merged

Use single quotes for readability in error assertions#17455
tomponline merged 2 commits intocanonical:mainfrom
kadinsayani:lxd-fix-17125-follow-up

Conversation

@kadinsayani
Copy link
Member

Follow-up to #17432.

Copy link

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

This PR improves readability of error message assertions in test files by switching from double-quoted to single-quoted strings. This change eliminates the need to escape internal double quotes within error messages, making the test assertions easier to read and maintain.

Changes:

  • Updated error assertion comparisons to use single quotes instead of double quotes
  • Modified test README to document the preference for single-quoted strings in error assertions

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/suites/storage_snapshots.sh Changed error message assertions to use single quotes for improved readability
test/suites/snapshots.sh Changed ERROR_MSG variable assignment to use single quotes
test/suites/serverconfig.sh Changed grep pattern assertions to use single quotes with proper variable interpolation
test/suites/clustering_waitready.sh Changed error message assertions to use single quotes, handling apostrophes with proper quoting
test/suites/clustering.sh Changed error message assertions to use single quotes with proper variable interpolation
test/README.md Added documentation recommending single-quoted strings for error assertions

Copy link
Member

@simondeziel simondeziel left a comment

Choose a reason for hiding this comment

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

Whenever there is a ' in the string or a variable we need to embed, the readability grain goes down the drain :/

It's a question of preference/taste but I'd use the single quoting option only when it avoids escaping.

Signed-off-by: Kadin Sayani <kadin.sayani@canonical.com>
Signed-off-by: Kadin Sayani <kadin.sayani@canonical.com>
@kadinsayani kadinsayani force-pushed the lxd-fix-17125-follow-up branch from d78d390 to 71da7a2 Compare January 21, 2026 18:50
@kadinsayani
Copy link
Member Author

kadinsayani commented Jan 21, 2026

Whenever there is a ' in the string or a variable we need to embed, the readability grain goes down the drain :/

It's a question of preference/taste but I'd use the single quoting option only when it avoids escaping.

aren'"'"'t is certainly not the most readable 😆

As per the new CLI guidelines, I believe we aren't supposed to have contractions in error messages, so those should say "are not". Furthermore, I think "have not" is more grammatically correct: "Cannot restore "node1" because some storage pools have not started yet".

"Cannot restore "node1" because some storage pools are stopped" might be even better.

@kadinsayani
Copy link
Member Author

Whenever there is a ' in the string or a variable we need to embed, the readability grain goes down the drain :/
It's a question of preference/taste but I'd use the single quoting option only when it avoids escaping.

aren'"'"'t is certainly not the most readable 😆

As per the new CLI guidelines, I believe we aren't supposed to have contractions in error messages, so those should say "are not". Furthermore, I think "have not" is more grammatically correct: "Cannot restore "node1" because some storage pools have not started yet".

"Cannot restore "node1" because some storage pools are stopped" might be even better.

#17460

Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Much nicer ta

@tomponline tomponline merged commit 474812e into canonical:main Jan 22, 2026
67 checks passed
@kadinsayani kadinsayani deleted the lxd-fix-17125-follow-up branch January 22, 2026 15:33
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.

4 participants