Skip to content

assert.InEpsilonSlice: include msgAndArgs on failed assertion #1454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
VladPetriv wants to merge 5 commits into
base: master
Choose a base branch
from
Open

assert.InEpsilonSlice: include msgAndArgs on failed assertion #1454

VladPetriv wants to merge 5 commits into from
+17 −2

Conversation

@VladPetriv
Copy link

@VladPetriv VladPetriv commented Aug 9, 2023

Summary

Added the msgAndArgs parameter to the InEpsilon function for better error reporting in case of failed assertion.

Related issues

Closes #1324

dolmen
dolmen requested changes Aug 11, 2023
View reviewed changes
Copy link
Collaborator

@dolmen dolmen left a comment

Choose a reason for hiding this comment

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

We also need a regression test.

I expect that the test will also be a good base for further improvement (in separate PRs) in reporting errors such as the index of the mismatch.

@VladPetriv
Copy link
Author

VladPetriv commented Aug 12, 2023

Hi @dolmen, thank you for the quick review!

We also need a regression test.

Can you provide more details about which regression tests you are referring to in the testify project? Thank you!

@dolmen
Copy link
Collaborator

dolmen commented Oct 10, 2023

Related: #1231 (also about InEpsilonSlice)

@dolmen dolmen added pkg-assert Change related to package testify/assert bug labels Oct 10, 2023
@dolmen
Copy link
Collaborator

dolmen commented Oct 10, 2023

@VladPetriv

Can you provide more details about which regression tests you are referring to in the testify project? Thank you!

The test that you didn't write!

We need to augment TestInEpsilonSlice to check that msgAndArgs is passed and reported in case of a failing test.

@dolmen dolmen mentioned this pull request Oct 10, 2023
Merged
@dolmen dolmen changed the title fix(InEpsilonSlice): Include msgAndArgs in InEpsilon for improved error visibility on failed assertion assert.InEpsilonSlice: include msgAndArgs on failed assertion Oct 10, 2023
@dolmen dolmen added the assert.InEpsilon About assert.InEpsilon and family label Oct 16, 2023
@VladPetriv VladPetriv force-pushed the fix/pass-additional-arguments branch from a2665e8 to 2be4cff Compare October 16, 2023 18:37
@VladPetriv VladPetriv requested a review from dolmen October 26, 2023 18:50
@dolmen
Copy link
Collaborator

dolmen commented Oct 30, 2023

This blocked by #1483 that must be merged first.

dolmen
dolmen requested changes Oct 30, 2023
View reviewed changes
@dolmen dolmen added the hacktoberfest-accepted Hacktoberfest label Oct 30, 2023
@VladPetriv VladPetriv requested a review from dolmen October 31, 2023 19:23
dolmen
dolmen requested changes Oct 31, 2023
View reviewed changes
assert/assertions_test.go Outdated

False(t, InEpsilonSlice(mockT, "", nil, 1), "Expected non numeral slices to fail")

inEpsilonSliceMockT := new(mockTestingT)
Copy link
Collaborator

@dolmen dolmen Oct 31, 2023

Choose a reason for hiding this comment

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

Rename the variable to mockT for consistency.

@VladPetriv VladPetriv requested a review from dolmen November 1, 2023 06:54
@justenstall
Copy link

justenstall commented Nov 18, 2025

Just ran into this bug, can this PR be picked back up since #1483 is merged?

@glesica
Copy link
Collaborator

glesica commented Nov 18, 2025

The merge conflicts are small, but I'm hesitant to touch them since I'm not in this codebase much any more.

@ccoVeille
Copy link
Collaborator

ccoVeille commented Nov 18, 2025

I can rebase and fix this.

@ccoVeille ccoVeille force-pushed the fix/pass-additional-arguments branch from 870ea86 to d810081 Compare November 18, 2025 19:11
@ccoVeille
Copy link
Collaborator

ccoVeille commented Nov 18, 2025

It's done, if someone can review and tell me

@ccoVeille ccoVeille force-pushed the fix/pass-additional-arguments branch from d810081 to 394bd1f Compare November 18, 2025 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dolmen dolmen Awaiting requested review from dolmen

@brackendawson brackendawson Awaiting requested review from brackendawson

@glesica glesica Awaiting requested review from glesica

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

assert.InEpsilon About assert.InEpsilon and family bug hacktoberfest-accepted Hacktoberfest pkg-assert Change related to package testify/assert

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

InEpsilonSlice doesn't append custom error message

5 participants

@VladPetriv @dolmen @justenstall @glesica @ccoVeille