Skip to content
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.

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

Add Helper() method in internal mocks and assert.CollectT #1423

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dolmen
Copy link
Collaborator

@dolmen dolmen commented Jul 13, 2023

Summary

Start to extend our definition of testing.TB to include its Helper() method. See #1422 for details.

Changes

  • Add an Helper() method on all internal mocks of testing.T.
  • Add an Helper() method on type assert.CollectT.

Motivation

These are steps 1 and 2 of #1422.

@dolmen dolmen self-assigned this Jul 13, 2023
@dolmen dolmen changed the title Add helper method in tests Add Helper() method in internal mocks and assert.CollectT Jul 13, 2023
@dolmen dolmen added pkg-mock Any issues related to Mock pkg-assert Change related to package testify/assert pkg-require Change related to package testify/require labels Jul 13, 2023
@dolmen dolmen added TB.Helper Related to hiding testify calls in stack using testing.TB.Helper assert.Eventually About assert.Eventually/EventuallyWithT labels Jul 31, 2023
@dolmen dolmen added the hacktoberfest-accepted Hacktoberfest label Oct 16, 2023
Add an Helper() method to all mocks in tests of package 'assert'.
Add Helper() method to CollectT like testing.T as we intend to add
Helper() to the assert.TestingT interface.
@@ -1874,6 +1874,9 @@ type CollectT struct {
errors []error
}

// Helper is like [testing.T.Helper] but does nothing.
func (CollectT) Helper() {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the newly merged captureTestingT also get a Helper() method? (Or, should captureTestingT be replaced with CollectT?)

Copy link
Collaborator

@brackendawson brackendawson left a comment

Choose a reason for hiding this comment

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

Given this doesn't change the interface I don't want to block progress, so approving.

But I think @craig65535 is right, ideally re-base this and add consolidate the new internal type using one of those approaches.

@brackendawson brackendawson modified the milestones: v1.8.5, v1.9.0, v1.9.1, v1.10 Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert.Eventually About assert.Eventually/EventuallyWithT hacktoberfest-accepted Hacktoberfest pkg-assert Change related to package testify/assert pkg-mock Any issues related to Mock pkg-require Change related to package testify/require ready-for-merge TB.Helper Related to hiding testify calls in stack using testing.TB.Helper
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants