fix: use %#v instead of %q in Subset/NotSubset error messages

[veeceey]

Summary

• Fixes NotSubset (and Subset) error messages that use %q format verb, which produces broken output for non-string types (e.g., %!q(bool=true) for booleans, '\x01' for integers)
• Changes to %#v (Go-syntax representation) to match how Subset already formats its "does not contain" failure messages
• Updates corresponding test expectations to reflect the corrected output format

Before (broken):

Error: [%!q(bool=true)] is a subset of [%!q(bool=true)]

After (fixed):

Error: []bool{true} is a subset of []bool{true}

Fixes #1800

Test plan

• All existing TestSubsetNotSubset tests pass with updated expectations
• TestNotSubsetNil passes unchanged
• TestNotSubsetWithSliceTooLongToPrint passes with updated expectations
• TestNotSubsetWithMapTooLongToPrint passes with updated expectations
• TestSubsetWithSliceTooLongToPrint and TestSubsetWithMapTooLongToPrint pass unchanged
• Full test suite (go test ./...) passes with no regressions

🤖 Generated with Claude Code

[veeceey]

Friendly ping - any chance someone could take a look at this when they get a chance? Happy to make any changes if needed.

[ccoVeille]

This sounds reasonable, but it's a known thing our test coverage on edge cases is low. So I wonder how this little change could affect existing code. But as it's only about the reporting message. I feel we are good.

Maybe there is a need to limit the changes to one that were failing. So a method could be added to return the old %q for things that can be reported with it, and %#v for others.

I might be wrong, and we are good, but I prefer to raise the point

[veeceey]

Thanks for the review @ccoVeille! Good call on scoping the changes — I can limit it to just the methods that were actually failing with the bad message. Want me to narrow it down?

[veeceey]

Narrowed the scope to only the methods that were producing confusing output. Good call on keeping the blast radius small.

[ccoVeille]

Thanks for the review. Good call on scoping the changes — I can limit it to just the methods that were actually failing with the bad message.

Narrowed the scope to only the methods that were producing confusing output. Good call on keeping the blast radius small.

Thanks for this change.

Here again the commit shows off the lack of tests we have in testify.

You were able to change this without having to change unit tests.

Could you please add unit tests that are missing ? I assume you have a good knowledge of the types this method handles.

[veeceey]

Absolutely, I can add unit tests for the affected methods. You're right that changing the formatting without touching tests is a red flag for coverage gaps. I'll add tests that exercise the different data types and edge cases these methods handle to make sure the new formatting works as expected.

[veeceey]

Added more unit tests to expand coverage for the error message formatting across different types:

• String slices -- verifies %#v still produces readable output for the one type %q handled well (e.g. []string{"hello"})
• Map-to-map with int keys -- exercises the map-to-map comparison path in NotSubset with non-string keys
• Subset failure with map of int keys -- exercises the map "does not contain" path in Subset
• Subset failure with uint slices -- covers the slice "does not contain" path with unsigned integers
• NotSubset with int32 slices -- covers a narrower integer type to verify no %!q artifacts

All tests pass, including the full suite (go test ./...). The test function TestSubsetNotSubsetErrorMessages now has 14 subtests covering bool, float64, uint, int32, byte, string, struct, and map types for both Subset and NotSubset error paths.

[Copilot]

Pull request overview

This PR fixes a bug in the NotSubset assertion error messages where the %q format verb was incorrectly used to format non-string collection types, producing broken output like %!q(bool=true) for booleans and '\x01' for integers. The fix changes the format verb from %q to %#v (Go-syntax representation), which already matches how the Subset function formats its "does not contain" failure messages.

Changes:

• Changed format verb from %q to %#v in three locations within the NotSubset function for "is a subset of" error messages
• Updated test expectations in TestSubsetNotSubset to reflect the corrected Go-syntax format
• Added comprehensive new test TestSubsetNotSubsetErrorMessages with 15 test cases covering various data types (bool, float64, uint, int32, byte, struct slices, and maps)
• Updated test expectations in TestNotSubsetWithSliceTooLongToPrint and TestNotSubsetWithMapTooLongToPrint for truncated output

Reviewed changes

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

File	Description
assert/assertions.go	Changed three error message format calls in NotSubset from %q to %#v to fix broken output for non-string types
assert/assertions_test.go	Updated test expectations to match new Go-syntax format and added comprehensive test coverage for error message formatting across multiple data types

---

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

[Copilot]

The format verb %q should be changed to %#v for consistency with the rest of the fix and to avoid broken output for non-string types. If a user passes a boolean or integer value where a slice/map is expected, this error message would display something like %!q(bool=true) has an unsupported type bool instead of a properly formatted error.

[Copilot]

The format verb %q should be changed to %#v for consistency with the rest of the fix and to avoid broken output for non-string types. If a user passes a boolean or integer value where a slice/map is expected, this error message would display something like %!q(bool=true) has an unsupported type bool instead of a properly formatted error.

[Copilot]

The format verb %q should be changed to %#v for consistency with the NotSubset fix and to avoid broken output for non-string types. If a user passes a boolean or integer value where a slice/map is expected, this error message would display something like %!q(bool=true) has an unsupported type bool instead of a properly formatted error.

[Copilot]

The format verb %q should be changed to %#v for consistency with the NotSubset fix and to avoid broken output for non-string types. If a user passes a boolean or integer value where a slice/map is expected, this error message would display something like %!q(bool=true) has an unsupported type bool instead of a properly formatted error.

[Copilot]

The format verb %q should be changed to %#v for consistency with the rest of the codebase and to avoid broken output for non-string types. If a user passes a boolean or integer value where a slice is expected, this error message would display something like %!q(bool=true) has an unsupported type bool, expecting array or slice instead of a properly formatted error. This is the same issue that was fixed in NotSubset's main error messages.

[ccoVeille]

@veeceey could you check at Copilot feedbacks ? Maybe there is Jo need, but just in case.

Please add tests if you changes some code

[ccoVeille]

You should mention the related issues too

[ccoVeille]

I think this is enough

Suggested change

	Contains(t, msg, "[]bool{true} is a subset of []bool{true, false}")
	// Ensure no broken %q formatting like %!q(bool=true)
	NotContains(t, msg, "%!q")
	Contains(t, msg, "[]bool{true} is a subset of []bool{true, false}")

The code may divert and this refers to something you remove, and that no longer exists once this PR will be merged.

For me checking, Contains is enough.

This comment apply to everything you added to this PR

[ccoVeille]

If we want to add a detection of bad formatting looking for "%!" would be OK if we consider it will detect:

• %!(MISSING)
• %!(EXTRA string=foo) and variation
• %!d(string=hello), %!q(bool=true)

But then I feel it should be part of testify test suite, and not limited to this PR, and the refactoring would be way larger than the scope of this PR and related issue