Skip to content

assert: Fail Subset/NotSubset when a map superset is missing elements - #1290

Closed
antichris wants to merge 1 commit into
stretchr:masterfrom
antichris:fix-1270
Closed

antichris wants to merge 1 commit into
stretchr:masterfrom
antichris:fix-1270

Conversation

@antichris

@antichris antichris commented Oct 31, 2022 •

Copy link
Copy Markdown

Summary

Avoid panics in assert.Subset/assert.NotSubset and properly fail the test when a superset map is missing elements from the subset map.

Changes

  • Check whether the value reflected at the subsetKey of the superset reflection listValue .isValid, before attempting to get its interface value for use in ObjectsAreEqual assertion.
  • Align the Subset failure message with the corresponding NotSubset failure message.

Motivation

When a subset map contains an item with a key that is not present in the superset map, attempts to reflect the interface of the value at the absent key cause panics, which return false on recovery but do not properly fail the test with a message.

Related issues

Closes #1270

When a subset map contains an item with a key that is not present in
the superset map, attempts to reflect the interface of the value at
the absent key cause panics, which return false on recovery but do not
properly fail the test with a message.

This also aligns the `Subset` failure message with the corresponding
`NotSubset` failure message.
@dolmen dolmen added bug pkg-assert Change related to package testify/assert must-rebase labels Jul 25, 2023
@dolmen
dolmen requested review from brackendawson and dolmen July 25, 2023 03:47
@st3penta

Copy link
Copy Markdown

Duplicate of #1261

@brackendawson

Copy link
Copy Markdown
Collaborator

Thank you for your efforts, I can confirm this was fixed in the other PR, it's no longer a problem on v1.8.4 (latest) or on master.

@antichris
antichris deleted the fix-1270 branch February 17, 2024 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug pkg-assert Change related to package testify/assert

Projects

None yet

Development

Successfully merging this pull request may close these issues.

assert.Subset doesn't fail tests

4 participants