Restructure test helpers: add expect/require, Fail utils, testframe #47
Conversation
WalkthroughReorganizes and extends the test framework: adds expect/require helpers (bool, nil, error, type), new test.Run/Runnable and testframe cleanup/parallel APIs, introduces test.Outcome, refactors matchers and reporters, removes legacy type/panic/nilness surfaces, and updates many tests and README docs. No user-facing API removals beyond those in diff. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant TestCode as Test code / Caller
participant TF as internal/testframe
participant T as *testing.T
participant Runner as Runnable
TestCode->>TF: testframe.Peek[*testing.T]() / MustPeek
TF-->>TestCode: *testing.T (or panic)
TestCode->>T: Helper()
TestCode->>Runner: r.Run()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas to focus review on:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
matchers/slices/contains-item.go (1)
11-16: Update remaining references to ContainsItemsMatcher
Remaining uses of the oldContainsItemsMatcherAPI were found in:
- match-slices.go (lines 46–47):
func ContainItems[T any] … slices.ContainsItemsMatcher[T]- matchers/slices/contains-items.go (type and methods)
- matchers/slices/contains-items_test.go (TestContainsItems)
Rename all occurrences to
ContainsItemMatcher(and update the helper fromContainItemstoContainItem).opt/value-as-string.go (1)
13-18: Consider documenting the AsDeclaration option.The
AsDeclarationoption (used on line 32) is supported but not documented in the function's comment. While this line wasn't changed in this PR, documenting all supported options would improve developer ergonomics.Consider adding documentation for the
AsDeclarationoption:// Supported options: // // opt.QuotedStrings(bool) // determines whether strings should be quoted // // (default is true). // // // // Has no effect if the value is not a string. +// +// opt.AsDeclaration(bool) // determines whether non-string values should be +// // formatted with type information (%#v). +// // +// // Has no effect if the value is a string.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (92)
README.md(20 hunks)bool.go(0 hunks)context_examples_test.go(0 hunks)context_test.go(0 hunks)emptiness_test.go(0 hunks)error.go(0 hunks)error_test.go(0 hunks)errors.go(1 hunks)expect/bool.go(1 hunks)expect/bool_test.go(1 hunks)expect/error.go(1 hunks)expect/error_test.go(1 hunks)expect/nil.go(1 hunks)expect/nil_test.go(1 hunks)expect/pkgdoc.go(1 hunks)expect/type.go(1 hunks)expect/type_test.go(1 hunks)expect_didOccur.go(0 hunks)expect_didOccur_test.go(0 hunks)expectation.go(1 hunks)expectation_test.go(6 hunks)fail.go(1 hunks)fail_test.go(1 hunks)internal/testcase/runner.go(1 hunks)internal/testcase/runner_test.go(2 hunks)internal/testframe/errors.go(1 hunks)internal/testframe/is-parallel.go(1 hunks)internal/testframe/is-parallel_test.go(1 hunks)internal/testframe/stack.go(1 hunks)internal/testframe/stack_test.go(1 hunks)internal/type-name.go(1 hunks)internal/type-name_test.go(1 hunks)match-boolean.go(1 hunks)match-boolean_test.go(8 hunks)match-bytes.go(1 hunks)match-bytes_test.go(2 hunks)match-context_test.go(1 hunks)match-emptiness.go(0 hunks)match-emptiness_test.go(1 hunks)match-equals.go(3 hunks)match-equals_test.go(1 hunks)match-errors-nil-and-panics.go(1 hunks)match-errors-nil-and-panics_test.go(1 hunks)match-length.go(1 hunks)match-length_test.go(1 hunks)match-typecheck.go(1 hunks)match-typecheck_test.go(1 hunks)matchers/bools/equal.go(1 hunks)matchers/nilness/matcher.go(1 hunks)matchers/nilness/matcher_test.go(1 hunks)matchers/panics/match-recovered_test.go(2 hunks)matchers/slices/contains-item.go(1 hunks)matchers/slices/contains-slice.go(1 hunks)matchers/slices/equals-slice.go(1 hunks)matchers/typecheck/match-error.go(1 hunks)matchers/typecheck/match-error_test.go(1 hunks)matchers/typecheck/matcher.go(2 hunks)mock-fn.go(1 hunks)nilness.go(0 hunks)nilness_test.go(0 hunks)opt/failure-report.go(1 hunks)opt/get.go(2 hunks)opt/opts.go(1 hunks)opt/opts_test.go(5 hunks)opt/value-as-string.go(1 hunks)panic.go(0 hunks)panic_test.go(0 hunks)parallel.go(2 hunks)parallel_test.go(1 hunks)pkgdocs.go(1 hunks)require/bool.go(1 hunks)require/bool_test.go(1 hunks)require/error.go(1 hunks)require/error_test.go(1 hunks)require/nil.go(1 hunks)require/nil_test.go(1 hunks)require/type.go(1 hunks)require/type_test.go(6 hunks)run.go(1 hunks)run_test.go(1 hunks)runnable-flaky.go(5 hunks)runnable-flaky_test.go(7 hunks)runnable-helper-tests.go(1 hunks)runnable-test.go(2 hunks)runnable-testcases.go(2 hunks)test-helper.go(13 hunks)test-helper_test.go(9 hunks)test/invalidate.go(3 hunks)test/outcome.go(1 hunks)test/outcome_test.go(1 hunks)testframe.go(1 hunks)type.go(0 hunks)
💤 Files with no reviewable changes (14)
- error.go
- bool.go
- type.go
- nilness.go
- context_test.go
- match-emptiness.go
- emptiness_test.go
- panic_test.go
- panic.go
- expect_didOccur_test.go
- error_test.go
- context_examples_test.go
- nilness_test.go
- expect_didOccur.go
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2025-06-15T03:06:27.000Z
Learnt from: deltics
PR: blugnu/test#30
File: test/example.go:13-14
Timestamp: 2025-06-15T03:06:27.000Z
Learning: The Example() function in test/example.go intentionally pushes testframe.example instances onto the stack without popping them. This is by design because: 1) The frames need to remain available for subsequent example code to use via T(), 2) Doc examples run in a single goroutine per package and are short-lived, 3) The memory overhead is negligible, and 4) The complexity of managing frame cleanup would introduce more risk than benefit.
Applied to files:
internal/testframe/stack.gointernal/testframe/stack_test.gotestframe.goREADME.md
📚 Learning: 2025-05-27T08:15:10.945Z
Learnt from: deltics
PR: blugnu/test#25
File: parallel.go:17-20
Timestamp: 2025-05-27T08:15:10.945Z
Learning: In the `isParallel` function in parallel.go, the check `if _, ok := T().(*testing.T); !ok` is intentional and checks the global test frame T() rather than the parameter t. This serves as a precondition - if T() is not a *testing.T, then parallel execution is not supported by the testing framework context, making it unnecessary to check the parallel state of the specific test instance parameter t.
Applied to files:
parallel.goparallel_test.gointernal/testframe/is-parallel_test.gorunnable-test.gointernal/testframe/is-parallel.goREADME.md
📚 Learning: 2025-06-13T22:45:11.744Z
Learnt from: deltics
PR: blugnu/test#30
File: opt/valueAsString.go:24-34
Timestamp: 2025-06-13T22:45:11.744Z
Learning: In opt/valueAsString.go, the ValueAsString function deliberately applies the AsDeclaration option only to non-string values. String values are formatted as strings and are subject only to the QuotedStrings option. The switch statement order is intentional - string cases come before AsDeclaration to ensure strings are never formatted with %#v even when AsDeclaration(true) is set.
Applied to files:
opt/value-as-string.go
📚 Learning: 2025-05-30T18:15:33.258Z
Learnt from: deltics
PR: blugnu/test#25
File: expect_isEmpty.go:230-236
Timestamp: 2025-05-30T18:15:33.258Z
Learning: The e.err() method in the expectation[T] type accepts an 'any' parameter with special handling for nil, string, or []string values. Single-line error messages use string while multi-line reports use []string, allowing for different formatting approaches in test failure reporting.
Applied to files:
expect/error.gomatch-errors-nil-and-panics_test.gomatch-errors-nil-and-panics.goexpect/error_test.gorequire/type_test.goREADME.mdexpect/nil_test.go
📚 Learning: 2025-05-30T18:55:32.543Z
Learnt from: deltics
PR: blugnu/test#25
File: expect_test.go:68-68
Timestamp: 2025-05-30T18:55:32.543Z
Learning: The `R.Expect` method in the `test` package automatically verifies the presence of the test filename in failure reports, eliminating the need for explicit `TestFilename()` calls in test assertions and simplifying both single-line and multi-line report expectations.
Applied to files:
expectation_test.gomatch-boolean_test.goREADME.mdtest-helper_test.gotest-helper.go
📚 Learning: 2025-05-30T19:01:33.695Z
Learnt from: deltics
PR: blugnu/test#25
File: matchers/contexts/valueMatcher.go:21-21
Timestamp: 2025-05-30T19:01:33.695Z
Learning: In heterogeneous matchers, when `MatchAny` is called by the framework, the type of the `got` parameter is guaranteed to match the type parameter `T` because the framework has already verified type compatibility through the `Match` method. This makes type assertions in `MatchAny` implementations safe.
Applied to files:
pkgdocs.go
📚 Learning: 2025-06-14T00:35:50.260Z
Learnt from: deltics
PR: blugnu/test#30
File: t_test.go:45-52
Timestamp: 2025-06-14T00:35:50.260Z
Learning: In the blugnu/test framework, `defer Expect(Panic(...)).DidOccur()` works by executing `Expect(Panic(...))` immediately to set up the panic expectation, while only deferring the `.DidOccur()` method call for verification after the panic occurs. This allows proper panic testing where the expectation is established before the panic and verification happens after.
Applied to files:
matchers/panics/match-recovered_test.gomatch-errors-nil-and-panics_test.gomatch-errors-nil-and-panics.goREADME.md
📚 Learning: 2024-11-07T23:05:45.944Z
Learnt from: deltics
PR: blugnu/test#23
File: context_test.go:12-17
Timestamp: 2024-11-07T23:05:45.944Z
Learning: The function `ContextKey` in `context_test.go` is a test fixture used only with known string values, so the unsafe type assertion `v.(string)` is acceptable.
Applied to files:
match-context_test.go
📚 Learning: 2025-06-15T03:58:31.152Z
Learnt from: deltics
PR: blugnu/test#30
File: README.md:0-0
Timestamp: 2025-06-15T03:58:31.152Z
Learning: When documenting functions in the blugnu/test package, prioritize showing typical usage patterns over technical function signatures. For example, With(t *testing.T) should be documented as accepting *testing.T even though it technically accepts the TestingT interface, because that's how users will actually call it.
Applied to files:
README.md
📚 Learning: 2025-05-27T07:47:57.910Z
Learnt from: deltics
PR: blugnu/test#25
File: matchers/maps/equal.go:11-17
Timestamp: 2025-05-27T07:47:57.910Z
Learning: In matchers/maps/utils.go, the compareFuncFor function includes a fallback mechanism that defaults to reflect.DeepEqual when no other suitable comparator is found from type interfaces, custom functions, or special cases.
Applied to files:
match-equals.go
🧬 Code graph analysis (62)
opt/get.go (1)
opt/get_test.go (1)
TestGet(9-46)
internal/testframe/errors.go (5)
test/nilFrame.go (1)
NilFrame(51-56)internal/testframe/nilFrame_test.go (2)
Cleanup(13-15)TestNilFrame(17-59)internal/testframe/nilFrame.go (1)
Cleanup(23-23)cleanup_test.go (1)
TestCleanup(10-43)cleanup.go (1)
Cleanup(10-15)
match-boolean.go (3)
matchers/bools/equal.go (1)
BooleanMatcher(11-13)matchers/bools/bools_test.go (1)
TestBooleans(9-58)bool_test.go (4)
TestBooleans(11-137)ExampleBeTrue(166-173)ExampleBeFalse(157-164)ExampleExpectTrue(148-155)
matchers/typecheck/matcher.go (1)
internal/type-name.go (1)
TypeName(15-48)
matchers/slices/equals-slice.go (8)
matchers/equal/equal.go (1)
OnTestFailure(25-27)expect_test.go (2)
OnTestFailure(53-55)OnTestFailure(62-64)matchers/bytes/equal.go (1)
OnTestFailure(43-80)matchers/maps/contains.go (1)
OnTestFailure(19-32)matchers/slices/containsItems.go (1)
OnTestFailure(35-44)matchers/strings/regex.go (1)
OnTestFailure(17-29)matchers/maps/equal.go (1)
OnTestFailure(19-39)matchers/mocks/matcher.go (1)
OnTestFailure(76-83)
internal/testframe/stack.go (3)
internal/testframe/nilFrame.go (1)
Cleanup(6-8)internal/testframe/errors.go (1)
ErrNoCleanupFunction(8-8)internal/testframe/nilFrame_test.go (1)
Cleanup(13-15)
internal/type-name_test.go (5)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)internal/type-name.go (1)
TypeName(15-48)
run.go (3)
test/example.go (1)
Run(77-89)run-test.go (1)
Run(19-37)run-helper-tests.go (1)
Run(27-51)
matchers/nilness/matcher_test.go (1)
nilness_test.go (1)
TestExpect_IsNotNil(49-78)
parallel.go (3)
internal/testframe/is-parallel.go (1)
IsParallel(15-26)internal/testframe/stack.go (1)
Peek(68-87)test/invalidate.go (1)
Invalid(71-86)
runnable-helper-tests.go (1)
test/outcome.go (1)
Passed(8-8)
require/error.go (3)
opt/opts.go (1)
Required(77-79)expect.go (1)
Require(88-97)type.go (1)
RequireType(71-77)
match-length.go (2)
matchers/length/matcher_test.go (1)
TestLength(9-142)emptiness.go (1)
HaveLen(95-97)
opt/value-as-string.go (2)
opt/valueAsString_test.go (1)
TestValueAsString(11-39)matchers/equal/deepEqual.go (1)
valueAsString(16-45)
expect/nil.go (2)
expectation.go (1)
Expect(20-29)match-errors-nil-and-panics.go (1)
BeNil(278-280)
internal/testframe/stack_test.go (3)
internal/testframe/nilFrame.go (1)
Cleanup(6-8)internal/testframe/stack.go (1)
PushWithCleanup(134-142)internal/testframe/errors.go (1)
ErrNoCleanupFunction(8-8)
internal/testframe/is-parallel_test.go (5)
testframe.go (1)
With(96-102)internal/testframe/is-parallel.go (1)
IsParallel(15-26)parallel.go (2)
IsParallel(10-12)Parallel(37-53)test/example.go (1)
Example(12-29)internal/testframe/stack.go (1)
Pop(96-109)
expect/error.go (3)
opt/failure-report.go (1)
FailureReport(13-13)internal/type-name.go (1)
TypeName(15-48)fail.go (1)
Fail(37-40)
require/bool.go (2)
expectation.go (1)
Require(48-57)match-boolean.go (2)
BeFalse(13-15)BeTrue(23-25)
internal/type-name.go (1)
opt/name.go (1)
Name(7-15)
runnable-testcases.go (2)
runnable-test.go (1)
T(28-31)internal/testcase/runner.go (3)
Registration(53-53)Runner(46-51)Flags(11-11)
match-typecheck_test.go (7)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-typecheck.go (1)
BeOfType(9-11)test-helper.go (1)
R(26-49)test/example.go (1)
Example(12-29)
expectation_test.go (10)
runnable-test.go (1)
T(28-31)testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-errors-nil-and-panics.go (1)
Panic(363-378)errors.go (2)
ErrInvalidArgument(9-9)ErrInvalidOperation(10-10)test-helper.go (1)
R(26-49)test/outcome.go (1)
Failed(9-9)opt/opts.go (1)
IgnoreReport(24-24)
testframe.go (1)
internal/testframe/stack.go (1)
PushWithCleanup(134-142)
test/outcome_test.go (6)
testframe.go (1)
With(96-102)test/outcome.go (4)
Outcome(5-5)Passed(8-8)Failed(9-9)Panicked(10-10)run.go (1)
Run(19-27)runnable-testcases.go (3)
Testcases(172-180)ForEach(109-115)Cases(47-53)expectation.go (1)
Expect(20-29)match-equals.go (1)
Equal(61-63)
require/error_test.go (8)
require/error.go (1)
Error(20-26)testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)fail.go (1)
Errorf(27-30)expectation.go (1)
Expect(20-29)match-equals.go (1)
Equal(61-63)test-helper.go (1)
R(26-49)
runnable-flaky.go (1)
test/outcome.go (3)
Passed(8-8)Outcome(5-5)Failed(9-9)
match-equals_test.go (7)
runnable-test.go (1)
T(28-31)testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-equals.go (2)
Equal(61-63)DeepEqual(29-31)test/example.go (1)
Example(12-29)
match-boolean_test.go (3)
opt/failure-report.go (1)
FailureReport(13-13)test-helper.go (1)
R(26-49)expectation.go (1)
Expect(20-29)
expectation.go (3)
fail.go (2)
FailIfNot(68-71)FailIf(44-64)opt/failure-report.go (1)
FailureReport(13-13)match-equals.go (1)
DeepEqual(29-31)
require/bool_test.go (2)
test/example.go (1)
Example(12-29)require/bool.go (2)
False(21-24)True(44-47)
fail.go (4)
opt/failure-report.go (2)
OnFailure(21-25)FailureReport(13-13)opt/get.go (1)
Get(10-19)expectation.go (1)
Expect(20-29)match-boolean.go (1)
BeFalse(13-15)
test/invalidate.go (2)
fail.go (1)
Errorf(27-30)internal/testframe/stack.go (1)
Peek(68-87)
require/nil.go (2)
expectation.go (1)
Require(48-57)match-errors-nil-and-panics.go (1)
BeNil(278-280)
matchers/panics/match-recovered_test.go (1)
test/outcome.go (1)
Panicked(10-10)
fail_test.go (6)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)fail.go (5)
Error(16-19)Errorf(27-30)Fail(37-40)FailIf(44-64)FailIfNot(68-71)test-helper.go (1)
R(26-49)expectation.go (1)
Expect(20-29)
runnable-flaky_test.go (1)
test/outcome.go (1)
Passed(8-8)
match-length_test.go (5)
testframe.go (1)
With(96-102)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-length.go (1)
HaveLen(26-28)test/example.go (1)
Example(12-29)
matchers/typecheck/match-error_test.go (6)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-errors-nil-and-panics.go (1)
BeError(195-197)test-helper.go (1)
R(26-49)
run_test.go (1)
test/outcome.go (1)
Failed(9-9)
match-errors-nil-and-panics_test.go (9)
testframe.go (1)
With(96-102)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-errors-nil-and-panics.go (3)
BeError(195-197)NilPanic(317-321)Panic(363-378)test-helper.go (1)
R(26-49)opt/failure-report.go (1)
FailureReport(13-13)matchers/panics/match-recovered.go (1)
Expected(53-55)opt/opts.go (1)
NoPanicExpected(36-36)test/example.go (1)
Example(12-29)
match-errors-nil-and-panics.go (8)
runnable-test.go (2)
T(28-31)Helper(22-24)matchers/panics/match-recovered.go (2)
Expected(53-55)MatchRecovered(41-51)test/invalidate.go (1)
Invalid(71-86)opt/opts.go (2)
NoPanicExpected(36-36)ToNotMatch(64-64)opt/failure-report.go (1)
FailureReport(13-13)opt/value-as-string.go (1)
ValueAsString(19-37)expectation.go (1)
Expect(20-29)matchers/typecheck/match-error.go (1)
MatchError(10-12)
runnable-test.go (3)
run.go (1)
Run(19-27)internal/testframe/stack.go (2)
MustPeek(54-59)PushWithCleanup(134-142)test/invalidate.go (1)
Invalid(71-86)
internal/testframe/is-parallel.go (2)
parallel.go (1)
IsParallel(10-12)internal/testframe/stack.go (1)
Peek(68-87)
expect/error_test.go (4)
expect/error.go (1)
Error(29-53)testframe.go (1)
With(96-102)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)test-helper.go (1)
R(26-49)
match-context_test.go (9)
runnable-test.go (2)
T(28-31)Test(69-71)testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-context.go (2)
HaveContextKey(19-21)HaveContextValue(46-51)test-helper.go (1)
R(26-49)opt/opts.go (1)
IgnoreReport(24-24)test/example.go (1)
Example(12-29)
require/type_test.go (3)
require/type.go (1)
Type(15-21)expectation.go (1)
Expect(20-29)test/example.go (1)
Example(12-29)
match-emptiness_test.go (6)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-emptiness.go (2)
BeEmpty(40-42)BeEmptyOrNil(73-75)test/example.go (1)
Example(12-29)
expect/type_test.go (5)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expect/type.go (1)
Type(24-51)test/example.go (1)
Example(12-29)
require/type.go (1)
opt/opts.go (1)
Required(77-79)
expect/bool.go (2)
expectation.go (1)
Expect(20-29)match-boolean.go (2)
BeFalse(13-15)BeTrue(23-25)
matchers/typecheck/match-error.go (3)
internal/type-name.go (1)
TypeName(15-48)opt/is-set.go (1)
IsSet(3-10)opt/opts.go (1)
ToNotMatch(64-64)
opt/opts_test.go (3)
expect/type.go (1)
Type(24-51)opt/opts.go (4)
ExactOrder(18-18)IsRequired(32-32)StackTrace(56-56)QuotedStrings(49-49)matchers/panics/stacktrace.go (1)
StackTrace(9-33)
test-helper_test.go (3)
test/outcome.go (4)
Panicked(10-10)Failed(9-9)Outcome(5-5)Passed(8-8)test-helper.go (1)
R(26-49)testframe.go (1)
T(42-44)
internal/testcase/runner_test.go (1)
test/outcome.go (1)
Passed(8-8)
expect/type.go (4)
internal/type-name.go (1)
TypeName(15-48)opt/failure-report.go (1)
FailureReport(13-13)expectation.go (1)
Expect(20-29)match-equals.go (1)
Equal(61-63)
expect/bool_test.go (2)
test/example.go (1)
Example(12-29)expect/bool.go (2)
False(21-24)True(42-45)
require/nil_test.go (2)
test/example.go (1)
Example(12-29)require/nil.go (2)
Nil(21-24)NotNil(42-45)
match-bytes_test.go (1)
runnable-test.go (1)
T(28-31)
expect/nil_test.go (2)
test/example.go (1)
Example(12-29)expect/nil.go (2)
Nil(21-24)NotNil(42-45)
match-equals.go (1)
matchers/equal/deepEqual.go (1)
DeepMatcher(12-14)
test-helper.go (10)
test/outcome.go (4)
Outcome(5-5)Panicked(10-10)Failed(9-9)Passed(8-8)expectation.go (1)
Expect(20-29)test/invalidate.go (1)
Invalid(71-86)match-slices.go (2)
ContainSlice(59-61)ContainItem(24-26)opt/is-set.go (1)
IsSet(3-10)opt/name.go (1)
Name(7-15)testframe.go (1)
T(42-44)opt/unset.go (1)
Unset(4-15)match-emptiness.go (1)
BeEmptyOrNil(73-75)opt/failure-report.go (1)
FailureReport(13-13)
🔇 Additional comments (58)
matchers/slices/equals-slice.go (1)
47-61: Doc comment now matches the exported method name.Update aligns with Go doc style; no further action needed.
matchers/panics/match-recovered_test.go (2)
9-9: LGTM!The import addition correctly supports the qualified identifier usage below. This improves code clarity by making the symbol's origin explicit.
33-33: LGTM!The change from
TestPanickedtotest.Panickedaligns with the PR's objective of consolidating test helpers. Using a qualified identifier improves maintainability by clearly indicating the symbol's source package.opt/get.go (2)
3-4: LGTM! Documentation improvement enhances clarity.The updated documentation clearly specifies that the function operates on a slice and that only the first matching value is significant. This prevents potential confusion about the function's behavior with multiple matches.
17-18: LGTM! More idiomatic zero-value construction.The refactor from
*new(T)tovar zero Tis a good improvement. Both approaches produce the same zero value, but the new approach is more idiomatic Go and slightly more readable.run_test.go (1)
190-190: LGTM! Improved clarity with explicit outcome specification.The change from a dot-imported constant to the explicitly qualified
test.Failedis correct and aligns with Go best practices. The explicit outcome specification is particularly valuable in this test context, as it clearly emphasizes that despite theskip: trueflag, the debug method should cause the test to run and fail rather than be skipped.mock-fn.go (1)
296-296: Inconsistency between code and AI-generated summary.The AI-generated enriched summary incorrectly states: "Updates only the documentation text for the public exported entity MockFn.RecordCall." However, the actual change is to the doc comment for
MockFn.ExpectationsWereMet()(line 296), notMockFn.RecordCall(). TheRecordCall()method doc comment begins at line 199 and remains unchanged.The updated documentation text itself is accurate and correctly describes the method's behavior.
match-context_test.go (1)
1-107: Well-structured test file with comprehensive coverage.The test file follows best practices:
- Uses typed keys for context values (prevents key collisions)
- Tests both positive and negative cases comprehensively
- Properly uses
opt.IgnoreReport(true)for expected failures- Examples include clear output demonstrating both passing and failing scenarios
- Consistent use of the
HelperTestspattern for scenario-based testingThe test coverage is thorough, checking:
- Key presence/absence
- Value presence/absence
- Value mismatch scenarios
match-bytes_test.go (3)
11-44: LGTM! Improved test function naming.The rename from
TestSliceOfBytestoTestEqualBytesbetter aligns with the matcher name and improves code clarity. The test structure is comprehensive, covering equality checks, negation, custom byte types, and option handling.
46-48: LGTM! Good organizational structure.The section marker clearly delineates the examples section from the tests, improving code navigation and readability.
50-64: LGTM! Well-structured example demonstrating error output.The example intentionally shows a failure case to document the error message format. The output clearly demonstrates:
- How byte differences are identified by index
- The visual hex comparison format with alignment indicators
This is valuable documentation for users of the
EqualBytesmatcher.internal/testcase/runner_test.go (2)
9-9: Qualified import from subpackage added alongside dot import.The addition of the qualified import
"github.com/blugnu/test/test"is necessary to access thePassedconstant. However, combining a dot import from the parent package (line 7) with a qualified import from a subpackage creates a mixed pattern where some symbols are unqualified and others require thetest.prefix.While this is valid Go code and appears to be part of a deliberate restructuring (per the PR objectives), verify that this pattern is consistently applied across other test files to maintain readability.
137-137: Usage of qualified constant is correct.The change from a dot-imported symbol to the qualified
test.Passedis more explicit and correctly references thePassedconstant from thegithub.com/blugnu/test/testsubpackage.match-length.go (2)
7-25: LGTM! Comprehensive documentation.The documentation clearly describes the matcher's behavior, supported types, nil handling, and failure cases. The example error message is particularly helpful.
26-28: Approve HaveLen constructorImplementation is correct and no duplicate
HaveLenfunctions were found.match-length_test.go (1)
33-68: LGTM! Comprehensive example.The example function provides excellent coverage of
HaveLenusage across multiple types (slice, map, string, array, channel) and scenarios (empty, nil, failing cases). The output documentation clearly shows expected error messages.runnable-flaky.go (4)
65-70: LGTM! Documentation and signature correctly updated.The function signature and documentation have been properly updated to use the public
test.Outcometype. The doc comment links are correct and follow Go documentation conventions.
80-88: LGTM! Inner function correctly refactored.The
handleOutcomeclosure has been properly updated to use the publictest.Outcometype andtest.Passedconstant. The logic remains sound and consistent with the outer function signature changes.
37-37: LGTM! Outcome checks and returns correctly updated.All comparisons against outcome values and return statements have been correctly updated to use the public constants
test.Passedandtest.Failed. The retry logic remains sound and consistent with the refactored type signatures.Also applies to: 108-109, 117-117
7-8: Avoid ambiguous import of nested “test” package
Importinggithub.com/blugnu/test/testinto packagetestcreates naming confusion and possible circular deps; confirm that thetest/testsub-package doesn’t import the parent and consider renaming it (e.g.testcore) for clarity.errors.go (3)
7-11: LGTM! Good organization improvement.Grouping general errors into a dedicated var block improves code organization and readability.
13-21: LGTM! Good organization improvement.Grouping mock and fake errors into a dedicated var block improves code organization and readability.
23-29: Rename fully propagated: no references toErrRecordingUnableToRedirectLoggerremain in the codebase.opt/failure-report.go (1)
15-15: LGTM! Documentation correction.The comment now correctly references the
OnTestFailuremethod name, improving documentation accuracy.matchers/nilness/matcher.go (1)
67-69: LGTM! Good optimization.The short-circuit return when the subject is nil in a NOT match scenario is a sensible optimization. It avoids unnecessary reflection and formatting operations when the failure message is straightforward. This change is consistent with the updated test expectations.
opt/opts_test.go (2)
7-7: LGTM! New import for migrated API.The import of
github.com/blugnu/test/expectsupports the migration fromExpectTypetoexpect.Type.
16-16: OldExpectTypeAPI successfully removed – no remaining definitions or usages found in Go files.matchers/slices/contains-slice.go (1)
35-35: LGTM! Documentation correction.The comment now correctly references the
OnTestFailuremethod name, improving documentation accuracy. This change is consistent with the documentation update inopt/failure-report.go.matchers/nilness/matcher_test.go (1)
122-122: All tests updated to the new error message. No occurrences of"expected not nil, got nil"remain in the codebase.parallel_test.go (1)
140-140: Implementation matches expected error message
No discrepancy found between the test and the code.runnable-helper-tests.go (1)
40-40: OldTestPassedconstant removed
No definitions or references of theTestPassedconstant remain.runnable-flaky_test.go (1)
8-8: LGTM! Consistent migration to new test framework constants.The changes correctly update file references to match the actual filename and migrate from legacy
TestPassedto the new publictest.Passedconstant. All test expectations remain consistent with the flaky test behavior.Also applies to: 23-23, 27-27, 31-31, 45-45, 49-49, 62-62, 73-73, 87-87, 100-100
opt/opts.go (1)
34-36: LGTM! Clear and descriptive rename.The rename from
NoPanictoNoPanicExpectedbetter conveys the sentinel's purpose in panic testing, and the updated comment provides clear context about its role in the testing mechanism.match-typecheck.go (1)
1-11: LGTM! Clean implementation with helpful documentation.The
BeOfType[T]()function follows Go idioms by returning an empty struct as a type marker. The documentation clearly guides users towardexpect.Typeorrequire.Typefor more advanced type assertions.testframe.go (1)
101-101: LGTM! Cleaner abstraction for frame management.The migration to
PushWithCleanup(t)encapsulates both the push and cleanup registration, reducing boilerplate and ensuring consistent cleanup behavior. SinceTestingTincludesCleanup(func()), this is type-safe.match-equals_test.go (1)
27-58: LGTM! Code reorganization with no functional changes.The relocation of
TestEqualandExampleDeepEqualimproves file organization without altering any test logic or behavior.pkgdocs.go (1)
1-50: LGTM! Excellent documentation interfaces.These documentation-only interfaces provide clear guidance on the matcher contract. The comprehensive comments explain the differences between
AnyMatcherandMatcher[T], their use cases, and the preference for type-safe matchers. The explicit note that these interfaces are not referenced by the package prevents confusion.match-typecheck_test.go (1)
1-64: LGTM! Comprehensive test coverage for BeOfType.The test suite thoroughly validates
BeOfType[T]()behavior:
- Positive and negative type matching scenarios
- Correct error message formatting
- Both
ShouldandShouldNotusage patterns- Clear documentation through the example function
All tests follow the established
HelperTestspattern used throughout the codebase.expectation_test.go (5)
293-339: LGTM! Thorough coverage of DidOccur scenarios.The
TestExpect_DidOccurtest comprehensively validates:
- Panic expectations with proper defer usage
- Error occurrence detection
- Invalid usage with unsupported types
All scenarios include appropriate assertions and clear failure messages.
341-418: LGTM! Complete validation of DidNotOccur behavior.The
TestExpect_DidNotOccurtest thoroughly covers:
- Successful non-occurrence of expected panics
- Detection of unexpected panics with clear error reporting
- Proper handling of
Panic(nil)with helpful error message- Error non-occurrence validation
- Invalid usage prevention
The test scenarios ensure robust error handling and clear diagnostic messages.
497-568: LGTM! Comprehensive IsNot test coverage.The
TestExpect_IsNottest validates all key scenarios:
- Nil and non-nil value handling
- Sentinel error comparisons
- Struct equality/inequality
- Proper error messages for each failure mode
The test suite ensures consistent behavior across different value types and edge cases.
588-588: LGTM! Clarified test intent with improved comments.The updated comments clearly distinguish between testing the behavior of
Should,ShouldNot, andToNotmethods versus validating matcher-specific output. Usingopt.IgnoreReport(true)is appropriate for these behavioral tests.Also applies to: 612-612, 645-645
662-664: LGTM! Improved file organization with section markers.The section marker comments help navigate the file and clearly separate test functions from example functions.
expect/pkgdoc.go (1)
1-4: LGTM!Standard package documentation pattern. The file serves its purpose of providing a package-level doc comment for the expect package.
matchers/bools/equal.go (1)
9-10: LGTM!Documentation improvement clarifies the role of BooleanMatcher in implementing the Matcher interface.
internal/testframe/errors.go (1)
8-8: LGTM!The new error constant follows the existing pattern and provides a clear error message for the PushWithCleanup use case.
require/type.go (1)
15-21: LGTM!The implementation follows the established require package pattern: delegates to
expect.Typewithopt.Required()appended, ensuring test failure is immediate rather than deferred. The documentation appropriately guides users towardBeOfTypematcher when the return value isn't needed.run.go (1)
9-27: LGTM!The
Runnableinterface andRunfunction provide a clean abstraction for executing test logic within the test frame. The panic message clearly indicates the context requirement, and the helper marking ensures proper call stack reporting.internal/testframe/is-parallel.go (1)
15-26: LGTM with brittleness caveat.The implementation correctly uses reflection to access
testing.T's internalisParallelstate. While reflection on unexported fields is inherently fragile and may break with Go version updates, this trade-off is accepted practice in test frameworks when no public API exists.Ensure this is covered by tests that will catch breakage if the internal structure changes in future Go versions.
parallel.go (1)
38-49: Keep the originalParallelargument intact
if t, ok := testframe.Peek[TestingT](); ok { ... }overwrites the function parametert(short declaration reuses the existing name). When a frame already sits on the stack—e.g. aftertest.Example()or when called from within another helper—you end up replacing the caller-suppliedtwith that peeked frame. The subsequentWith(t)andt.Parallel()then register cleanup and schedule parallelism on the wrong object, leaking the real test frame and breaking parallel setup.Preserve the original argument by assigning the peeked value to a different variable:
- if t, ok := testframe.Peek[TestingT](); ok { - t.Helper() + if helper, ok := testframe.Peek[TestingT](); ok { + helper.Helper() }⛔ Skipped due to learnings
Learnt from: deltics PR: blugnu/test#25 File: parallel.go:17-20 Timestamp: 2025-05-27T08:15:10.945Z Learning: In the `isParallel` function in parallel.go, the check `if _, ok := T().(*testing.T); !ok` is intentional and checks the global test frame T() rather than the parameter t. This serves as a precondition - if T() is not a *testing.T, then parallel execution is not supported by the testing framework context, making it unnecessary to check the parallel state of the specific test instance parameter t.internal/type-name_test.go (1)
10-79: LGTM! Comprehensive test coverage.The test scenarios thoroughly cover the TypeName function across a wide range of type cases including basic types, pointers, named types, interfaces (including nil), and anonymous types. The test structure is clear and the expected outputs are correct.
expect/bool.go (2)
42-45: Same opts duplication concern applies here.Similar to the
Falsefunction, verify whether passingoptsto bothtest.Expectand.To()is necessary.
21-24: Do not consolidateoptsusages.Expectextracts naming and required flags;Toforwards opts to the matcher and failure logic—both are necessary.Likely an incorrect or invalid review comment.
match-boolean.go (1)
1-25: LGTM! Clean matcher implementation.The boolean matcher factories are well-documented and correctly return
BooleanMatcherinstances with appropriateExpectedvalues. The implementation is simple and follows the established matcher pattern.matchers/typecheck/matcher.go (1)
36-37: LGTM! Excellent refactoring.Replacing the reflect-based type name extraction with
internal.TypeName[T]()simplifies the code significantly and provides a consistent approach to type naming across the codebase.require/error_test.go (1)
20-73: LGTM! Well-structured tests.The test scenarios comprehensively cover the
require.Error[T]function:
- Successful type assertion with wrapped error
- Type mismatch failure
- Nil error handling
Each scenario properly validates both the behavior and the error reporting. The use of
HelperTestsis appropriate for testing test helpers.internal/type-name.go (1)
15-48: LGTM with a minor observation!The
TypeNameimplementation is well-designed with clear fallback logic for edge cases. The comments effectively explain the trade-offs between using zero values and reflection.One observation: Line 40 calls
fn(t)solely "to ensure it is considered 'covered' by tests." This is unusual—production code typically shouldn't contain logic purely for test coverage metrics. However, since this is a NO-OP call with negligible runtime cost and the comment is explicit about its purpose, it's acceptable in this context.expect/type.go (1)
32-45: Well-structured mismatch diagnostics.The custom failure report covers both expected and actual type names, which keeps Type-based assertions easy to debug. Nice touch.
There was a problem hiding this comment.
Actionable comments posted: 25
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
README.md (3)
915-919: Fix reference to standard library subtest helperThe standard library exposes
Runas a method on*testing.T, not atest.Run()function. Please update the wording so readers aren’t pointed at a non-existent helper.-This type also provides a way to run subtests using the `test.Run()` function, which +This type also provides a way to run subtests using the `t.Run()` method, which
1000-1002: Clarify sentence about debug-only executionThis sentence drops the key verb, so it reads awkwardly. Adding “debugged” restores the intended meaning.
-When one or more test cases are, only the debug test cases are run +When one or more test cases are debugged, only the debug test cases are run
1266-1279: Align scenario slice type with HelperTests APIEarlier you explain that
HelperTestsconsumes[]HelperScenario, but this example switches to[]TestScenario. That inconsistency will confuse users copying the sample. Please keep the type name consistent.- scenarios := []TestScenario{ + scenarios := []HelperScenario{matchers/slices/contains-item.go (1)
11-16: Rename and remove legacy plural matcherThe singular
ContainsItemMatcherrename is incomplete. Update these remaining references:
- match-slices.go:46-47: rename
func ContainItems[…](items []T) slices.ContainsItemsMatcher[T]tofunc ContainItem[…](item T) slices.ContainsItemMatcher[T]- Remove
matchers/slices/contains-items.go(legacy plural matcher) and replace any imports or uses ofContainsItemsMatcherwithContainsItemMatcher- Re-run
rg -n 'ContainsItemsMatcher' --type goto confirm no leftoversrun.go (1)
20-27: Handle nil Runnable argumentLine 26 will panic with a nil-pointer dereference if callers accidentally pass a nil
Runnable. This is a user-facing API, so we should surface a clear error instead of letting the runtime crash. Add an explicit nil check before invokingr.Run().t.Helper() + if r == nil { + panic("ERROR: test.Run() requires a non-nil Runnable") + } r.Run()
♻️ Duplicate comments (1)
require/type_test.go (1)
101-127: Example text and usage align with require.Type (resolved)Intro comment and sample now reference require.Type and behavior correctly. This addresses the earlier “RequireType” naming mismatch.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (94)
README.md(21 hunks)bool.go(0 hunks)context_examples_test.go(0 hunks)context_test.go(0 hunks)emptiness_test.go(0 hunks)error.go(0 hunks)error_test.go(0 hunks)errors.go(1 hunks)expect/bool.go(1 hunks)expect/bool_test.go(1 hunks)expect/error.go(1 hunks)expect/error_test.go(1 hunks)expect/nil.go(1 hunks)expect/nil_test.go(1 hunks)expect/pkgdoc.go(1 hunks)expect/type.go(1 hunks)expect/type_test.go(1 hunks)expect_didOccur.go(0 hunks)expect_didOccur_test.go(0 hunks)expectation.go(1 hunks)expectation_test.go(6 hunks)fail.go(1 hunks)fail_test.go(1 hunks)internal/testcase/runner.go(1 hunks)internal/testcase/runner_test.go(2 hunks)internal/testframe/errors.go(1 hunks)internal/testframe/is-parallel.go(1 hunks)internal/testframe/is-parallel_test.go(1 hunks)internal/testframe/stack.go(1 hunks)internal/testframe/stack_test.go(1 hunks)internal/type-name.go(1 hunks)internal/type-name_test.go(1 hunks)match-boolean.go(1 hunks)match-boolean_test.go(8 hunks)match-bytes.go(1 hunks)match-bytes_test.go(2 hunks)match-context_test.go(1 hunks)match-emptiness.go(0 hunks)match-emptiness_test.go(1 hunks)match-equals.go(3 hunks)match-equals_test.go(1 hunks)match-errors-nil-and-panics.go(1 hunks)match-errors-nil-and-panics_test.go(1 hunks)match-length.go(1 hunks)match-length_test.go(1 hunks)match-typecheck.go(1 hunks)match-typecheck_test.go(1 hunks)matchers/bools/equal.go(1 hunks)matchers/nilness/matcher.go(1 hunks)matchers/nilness/matcher_test.go(1 hunks)matchers/panics/match-recovered_test.go(2 hunks)matchers/slices/contains-item.go(1 hunks)matchers/slices/contains-slice.go(1 hunks)matchers/slices/equals-slice.go(1 hunks)matchers/typecheck/match-error.go(1 hunks)matchers/typecheck/match-error_test.go(1 hunks)matchers/typecheck/matcher.go(2 hunks)mock-fn.go(1 hunks)nilness.go(0 hunks)nilness_test.go(0 hunks)opt/failure-report.go(1 hunks)opt/get.go(2 hunks)opt/opts.go(1 hunks)opt/opts_test.go(5 hunks)opt/value-as-string.go(1 hunks)panic.go(0 hunks)panic_test.go(0 hunks)parallel.go(2 hunks)parallel_test.go(1 hunks)pkgdocs.go(1 hunks)require/bool.go(1 hunks)require/bool_test.go(1 hunks)require/error.go(1 hunks)require/error_test.go(1 hunks)require/nil.go(1 hunks)require/nil_test.go(1 hunks)require/pkgdoc.go(1 hunks)require/type.go(1 hunks)require/type_test.go(1 hunks)run.go(1 hunks)run_test.go(1 hunks)runnable-flaky.go(5 hunks)runnable-flaky_test.go(7 hunks)runnable-helper-tests.go(1 hunks)runnable-test.go(2 hunks)runnable-testcases.go(2 hunks)test-helper.go(13 hunks)test-helper_test.go(9 hunks)test/invalidate.go(3 hunks)test/outcome.go(1 hunks)test/outcome_test.go(1 hunks)testframe.go(1 hunks)type.go(0 hunks)type_test.go(0 hunks)
💤 Files with no reviewable changes (15)
- error_test.go
- emptiness_test.go
- expect_didOccur_test.go
- panic.go
- nilness_test.go
- expect_didOccur.go
- nilness.go
- error.go
- bool.go
- match-emptiness.go
- panic_test.go
- type_test.go
- context_test.go
- context_examples_test.go
- type.go
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-06-15T03:06:27.000Z
Learnt from: deltics
PR: blugnu/test#30
File: test/example.go:13-14
Timestamp: 2025-06-15T03:06:27.000Z
Learning: The Example() function in test/example.go intentionally pushes testframe.example instances onto the stack without popping them. This is by design because: 1) The frames need to remain available for subsequent example code to use via T(), 2) Doc examples run in a single goroutine per package and are short-lived, 3) The memory overhead is negligible, and 4) The complexity of managing frame cleanup would introduce more risk than benefit.
Applied to files:
internal/testframe/stack_test.gointernal/testframe/stack.goREADME.mdtestframe.go
📚 Learning: 2025-05-27T08:15:10.945Z
Learnt from: deltics
PR: blugnu/test#25
File: parallel.go:17-20
Timestamp: 2025-05-27T08:15:10.945Z
Learning: In the `isParallel` function in parallel.go, the check `if _, ok := T().(*testing.T); !ok` is intentional and checks the global test frame T() rather than the parameter t. This serves as a precondition - if T() is not a *testing.T, then parallel execution is not supported by the testing framework context, making it unnecessary to check the parallel state of the specific test instance parameter t.
Applied to files:
parallel.goREADME.mdparallel_test.gorunnable-test.gointernal/testframe/is-parallel.gointernal/testframe/is-parallel_test.go
📚 Learning: 2025-05-30T18:15:33.258Z
Learnt from: deltics
PR: blugnu/test#25
File: expect_isEmpty.go:230-236
Timestamp: 2025-05-30T18:15:33.258Z
Learning: The e.err() method in the expectation[T] type accepts an 'any' parameter with special handling for nil, string, or []string values. Single-line error messages use string while multi-line reports use []string, allowing for different formatting approaches in test failure reporting.
Applied to files:
expect/error.goexpect/nil_test.goREADME.mdmatch-errors-nil-and-panics_test.gomatch-errors-nil-and-panics.goexpect/error_test.go
📚 Learning: 2025-05-30T19:01:33.695Z
Learnt from: deltics
PR: blugnu/test#25
File: matchers/contexts/valueMatcher.go:21-21
Timestamp: 2025-05-30T19:01:33.695Z
Learning: In heterogeneous matchers, when `MatchAny` is called by the framework, the type of the `got` parameter is guaranteed to match the type parameter `T` because the framework has already verified type compatibility through the `Match` method. This makes type assertions in `MatchAny` implementations safe.
Applied to files:
pkgdocs.go
📚 Learning: 2024-11-07T23:05:45.944Z
Learnt from: deltics
PR: blugnu/test#23
File: context_test.go:12-17
Timestamp: 2024-11-07T23:05:45.944Z
Learning: The function `ContextKey` in `context_test.go` is a test fixture used only with known string values, so the unsafe type assertion `v.(string)` is acceptable.
Applied to files:
match-context_test.go
📚 Learning: 2025-10-04T22:53:15.837Z
Learnt from: deltics
PR: blugnu/test#47
File: require/error.go:21-21
Timestamp: 2025-10-04T22:53:15.837Z
Learning: In the `require` and `expect` packages, use `test.T()` instead of `test.GetT()` to retrieve the test instance. The `GetT()` function is specifically provided for generic functions that have a type parameter named `T`, which would shadow the `T()` function from the test package. Since these packages cannot use dot-imports and their generic functions use different type parameter names (e.g., `E` for Error), there is no naming conflict with `T()`.
Applied to files:
expect/type.goREADME.mdexpect/type_test.gotest-helper_test.gorequire/type.gorequire/error.gorequire/type_test.go
📚 Learning: 2025-06-14T00:35:50.260Z
Learnt from: deltics
PR: blugnu/test#30
File: t_test.go:45-52
Timestamp: 2025-06-14T00:35:50.260Z
Learning: In the blugnu/test framework, `defer Expect(Panic(...)).DidOccur()` works by executing `Expect(Panic(...))` immediately to set up the panic expectation, while only deferring the `.DidOccur()` method call for verification after the panic occurs. This allows proper panic testing where the expectation is established before the panic and verification happens after.
Applied to files:
matchers/panics/match-recovered_test.goREADME.mdmatch-errors-nil-and-panics_test.gomatch-errors-nil-and-panics.go
📚 Learning: 2025-06-15T03:58:31.152Z
Learnt from: deltics
PR: blugnu/test#30
File: README.md:0-0
Timestamp: 2025-06-15T03:58:31.152Z
Learning: When documenting functions in the blugnu/test package, prioritize showing typical usage patterns over technical function signatures. For example, With(t *testing.T) should be documented as accepting *testing.T even though it technically accepts the TestingT interface, because that's how users will actually call it.
Applied to files:
README.md
📚 Learning: 2025-05-30T18:55:32.543Z
Learnt from: deltics
PR: blugnu/test#25
File: expect_test.go:68-68
Timestamp: 2025-05-30T18:55:32.543Z
Learning: The `R.Expect` method in the `test` package automatically verifies the presence of the test filename in failure reports, eliminating the need for explicit `TestFilename()` calls in test assertions and simplifying both single-line and multi-line report expectations.
Applied to files:
README.mdmatch-boolean_test.goexpectation_test.gotest-helper.gotest-helper_test.go
📚 Learning: 2025-06-13T22:45:11.744Z
Learnt from: deltics
PR: blugnu/test#30
File: opt/valueAsString.go:24-34
Timestamp: 2025-06-13T22:45:11.744Z
Learning: In opt/valueAsString.go, the ValueAsString function deliberately applies the AsDeclaration option only to non-string values. String values are formatted as strings and are subject only to the QuotedStrings option. The switch statement order is intentional - string cases come before AsDeclaration to ensure strings are never formatted with %#v even when AsDeclaration(true) is set.
Applied to files:
opt/value-as-string.go
📚 Learning: 2025-05-27T07:47:57.910Z
Learnt from: deltics
PR: blugnu/test#25
File: matchers/maps/equal.go:11-17
Timestamp: 2025-05-27T07:47:57.910Z
Learning: In matchers/maps/utils.go, the compareFuncFor function includes a fallback mechanism that defaults to reflect.DeepEqual when no other suitable comparator is found from type interfaces, custom functions, or special cases.
Applied to files:
match-equals.go
🧬 Code graph analysis (58)
internal/testframe/stack_test.go (3)
internal/testframe/nilFrame.go (1)
Cleanup(6-8)internal/testframe/stack.go (1)
PushWithCleanup(134-142)internal/testframe/errors.go (1)
ErrNoCleanupFunction(8-8)
run_test.go (2)
test/outcome.go (1)
Failed(9-9)test/example.go (3)
Failed(94-94)Fail(91-91)FailNow(92-92)
expect/nil.go (2)
match-errors-nil-and-panics.go (1)
BeNil(278-280)nilness.go (2)
IsNil(29-32)IsNotNil(48-51)
matchers/nilness/matcher_test.go (1)
nilness_test.go (2)
TestExpect_IsNil(11-47)TestExpect_IsNotNil(49-78)
matchers/slices/equals-slice.go (4)
matchers/equal/equal.go (1)
OnTestFailure(25-27)matchers/maps/equal.go (1)
OnTestFailure(19-39)matchers/slices/containsItems.go (1)
OnTestFailure(35-44)matchers/slices/containsSlice.go (1)
OnTestFailure(36-45)
expect/bool.go (3)
expectation.go (1)
Expect(20-29)match-boolean.go (2)
BeFalse(13-15)BeTrue(23-25)bool.go (2)
ExpectFalse(41-44)ExpectTrue(62-65)
internal/testframe/stack.go (2)
internal/testframe/nilFrame.go (1)
Cleanup(6-8)internal/testframe/errors.go (1)
ErrNoCleanupFunction(8-8)
matchers/slices/contains-slice.go (7)
matchers/slices/containsItems.go (1)
OnTestFailure(35-44)matchers/bytes/equal.go (1)
OnTestFailure(43-80)expect_test.go (2)
OnTestFailure(53-55)OnTestFailure(62-64)matchers/maps/contains.go (1)
OnTestFailure(19-32)matchers/slices/equalsSlice.go (1)
OnTestFailure(48-62)matchers/slices/containsItem.go (1)
OnTestFailure(47-66)matchers/strings/contains.go (1)
OnTestFailure(21-39)
mock-fn.go (2)
mockFn_test.go (1)
TestMockFnExpectationsWereMet(148-236)mockFn.go (2)
RecordCall(243-294)ExpectationsWereMet(308-329)
parallel.go (4)
internal/testframe/is-parallel.go (1)
IsParallel(15-26)internal/testframe/stack.go (1)
Peek(68-87)testframe.go (1)
TestingT(9-24)test/invalidate.go (1)
Invalid(71-86)
expect/error.go (3)
opt/failure-report.go (1)
FailureReport(13-13)internal/type-name.go (1)
TypeName(15-48)fail.go (1)
Fail(37-40)
match-typecheck_test.go (8)
runnable-test.go (1)
T(28-31)testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-typecheck.go (1)
BeOfType(9-11)test-helper.go (1)
R(26-49)test/example.go (1)
Example(12-29)
expect/nil_test.go (2)
test/example.go (1)
Example(12-29)expect/nil.go (2)
Nil(21-24)NotNil(42-45)
require/nil.go (2)
expectation.go (1)
Require(48-57)match-errors-nil-and-panics.go (1)
BeNil(278-280)
internal/type-name_test.go (4)
testframe.go (1)
With(96-102)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)internal/type-name.go (1)
TypeName(15-48)
match-context_test.go (9)
runnable-test.go (2)
T(28-31)Test(69-71)testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-context.go (2)
HaveContextKey(19-21)HaveContextValue(46-51)test-helper.go (1)
R(26-49)opt/opts.go (1)
IgnoreReport(24-24)test/example.go (1)
Example(12-29)
expect/type.go (4)
internal/type-name.go (1)
TypeName(15-48)opt/failure-report.go (1)
FailureReport(13-13)expectation.go (1)
Expect(20-29)match-equals.go (1)
Equal(61-63)
matchers/typecheck/matcher.go (1)
internal/type-name.go (1)
TypeName(15-48)
matchers/panics/match-recovered_test.go (1)
test/outcome.go (1)
Panicked(10-10)
matchers/typecheck/match-error.go (3)
internal/type-name.go (1)
TypeName(15-48)opt/is-set.go (1)
IsSet(3-10)opt/opts.go (1)
ToNotMatch(64-64)
require/nil_test.go (2)
test/example.go (1)
Example(12-29)require/nil.go (2)
Nil(21-24)NotNil(42-45)
matchers/typecheck/match-error_test.go (6)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-errors-nil-and-panics.go (1)
BeError(195-197)test-helper.go (1)
R(26-49)
runnable-test.go (3)
run.go (1)
Run(19-27)internal/testframe/stack.go (2)
MustPeek(54-59)PushWithCleanup(134-142)test/invalidate.go (1)
Invalid(71-86)
runnable-helper-tests.go (1)
test/outcome.go (1)
Passed(8-8)
match-errors-nil-and-panics_test.go (9)
runnable-test.go (1)
T(28-31)testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-errors-nil-and-panics.go (3)
BeError(195-197)NilPanic(291-293)Panic(335-350)matchers/panics/match-recovered.go (1)
Expected(53-55)opt/opts.go (2)
NoPanicExpected(36-36)NoStackTrace(72-74)test/example.go (1)
Example(12-29)
runnable-testcases.go (2)
runnable-test.go (1)
T(28-31)internal/testcase/runner.go (3)
Registration(53-53)Runner(46-51)Flags(11-11)
match-boolean_test.go (3)
opt/failure-report.go (1)
FailureReport(13-13)test-helper.go (1)
R(26-49)expectation.go (1)
Expect(20-29)
runnable-flaky_test.go (1)
test/outcome.go (1)
Passed(8-8)
match-bytes_test.go (1)
runnable-test.go (1)
T(28-31)
match-errors-nil-and-panics.go (6)
matchers/panics/match-recovered.go (2)
Expected(53-55)MatchRecovered(41-51)test/invalidate.go (1)
Invalid(71-86)opt/opts.go (2)
NoPanicExpected(36-36)ToNotMatch(64-64)opt/failure-report.go (1)
FailureReport(13-13)opt/value-as-string.go (1)
ValueAsString(20-38)matchers/typecheck/match-error.go (1)
MatchError(10-12)
expect/type_test.go (5)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expect/type.go (1)
Type(24-51)test/example.go (1)
Example(12-29)
internal/testcase/runner_test.go (1)
test/outcome.go (1)
Passed(8-8)
expect/error_test.go (8)
expect/error.go (1)
Error(29-53)testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-boolean.go (2)
BeTrue(23-25)BeFalse(13-15)match-equals.go (1)
Equal(61-63)test-helper.go (1)
R(26-49)
require/bool.go (2)
expectation.go (1)
Require(48-57)match-boolean.go (2)
BeFalse(13-15)BeTrue(23-25)
expect/bool_test.go (2)
test/example.go (1)
Example(12-29)expect/bool.go (2)
False(21-24)True(42-45)
expectation_test.go (9)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-errors-nil-and-panics.go (1)
Panic(335-350)errors.go (2)
ErrInvalidArgument(9-9)ErrInvalidOperation(10-10)test-helper.go (1)
R(26-49)test/outcome.go (1)
Failed(9-9)opt/opts.go (1)
IgnoreReport(24-24)
test-helper.go (12)
test/outcome.go (4)
Outcome(5-5)Panicked(10-10)Failed(9-9)Passed(8-8)expectation.go (1)
Expect(20-29)test/invalidate.go (1)
Invalid(71-86)match-slices.go (2)
ContainSlice(59-61)ContainItem(24-26)opt/opts.go (3)
UnquotedStrings(82-84)IgnoreReport(24-24)IsRequired(32-32)opt/is-set.go (1)
IsSet(3-10)opt/name.go (1)
Name(7-15)testframe.go (1)
T(42-44)opt/unset.go (1)
Unset(4-15)match-emptiness.go (1)
BeEmptyOrNil(73-75)opt/failure-report.go (1)
FailureReport(13-13)record.go (1)
Record(30-40)
require/error_test.go (5)
require/error.go (1)
Error(20-26)testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)
test-helper_test.go (3)
test/outcome.go (4)
Panicked(10-10)Failed(9-9)Outcome(5-5)Passed(8-8)test-helper.go (1)
R(26-49)testframe.go (1)
T(42-44)
opt/opts_test.go (3)
expect/type.go (1)
Type(24-51)opt/opts.go (4)
ExactOrder(18-18)IsRequired(32-32)StackTrace(56-56)QuotedStrings(49-49)matchers/panics/stacktrace.go (1)
StackTrace(9-33)
fail_test.go (6)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)fail.go (5)
Error(16-19)Errorf(27-30)Fail(37-40)FailIf(44-64)FailIfNot(68-71)test-helper.go (1)
R(26-49)expectation.go (1)
Expect(20-29)
internal/testframe/is-parallel.go (2)
parallel.go (1)
IsParallel(10-12)internal/testframe/stack.go (1)
Peek(68-87)
match-boolean.go (1)
matchers/bools/equal.go (1)
BooleanMatcher(11-13)
expectation.go (5)
runnable-test.go (2)
T(28-31)Helper(22-24)opt/get.go (1)
Get(10-19)opt/failure-report.go (2)
FailureReport(13-13)OnFailure(21-25)fail.go (3)
Fail(37-40)FailIfNot(68-71)FailIf(44-64)match-equals.go (1)
DeepEqual(29-31)
require/bool_test.go (2)
test/example.go (1)
Example(12-29)require/bool.go (2)
False(21-24)True(44-47)
match-length_test.go (5)
testframe.go (1)
With(96-102)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-length.go (1)
HaveLen(26-28)test/example.go (1)
Example(12-29)
match-equals.go (1)
matchers/equal/deepEqual.go (1)
DeepMatcher(12-14)
fail.go (4)
opt/failure-report.go (2)
OnFailure(21-25)FailureReport(13-13)opt/get.go (1)
Get(10-19)expectation.go (1)
Expect(20-29)match-boolean.go (1)
BeFalse(13-15)
internal/testframe/is-parallel_test.go (6)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)internal/testframe/is-parallel.go (1)
IsParallel(15-26)parallel.go (2)
IsParallel(10-12)Parallel(37-53)test/example.go (1)
Example(12-29)internal/testframe/stack.go (1)
Pop(96-109)
require/type.go (1)
opt/opts.go (1)
Required(77-79)
test/invalidate.go (2)
fail.go (1)
Errorf(27-30)internal/testframe/stack.go (1)
Peek(68-87)
require/error.go (1)
opt/opts.go (1)
Required(77-79)
match-emptiness_test.go (6)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-emptiness.go (2)
BeEmpty(40-42)BeEmptyOrNil(73-75)test/example.go (1)
Example(12-29)
test/outcome_test.go (5)
testframe.go (1)
With(96-102)test/outcome.go (4)
Outcome(5-5)Passed(8-8)Failed(9-9)Panicked(10-10)runnable-testcases.go (3)
Testcases(172-180)ForEach(109-115)Cases(47-53)expectation.go (1)
Expect(20-29)match-equals.go (1)
Equal(61-63)
runnable-flaky.go (1)
test/outcome.go (3)
Passed(8-8)Outcome(5-5)Failed(9-9)
testframe.go (1)
internal/testframe/stack.go (1)
PushWithCleanup(134-142)
require/type_test.go (6)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)require/type.go (1)
Type(15-21)test-helper.go (1)
R(26-49)test/example.go (1)
Example(12-29)
match-equals_test.go (7)
runnable-test.go (1)
T(28-31)testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(20-29)match-equals.go (2)
Equal(61-63)DeepEqual(29-31)test/example.go (1)
Example(12-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: module-qa / qa / sonarcloud: perform analysis / sonarcloud
| case any(expected) != nil && any(e.subject) == nil: | ||
| if _, ok := opt.Get[opt.FailureReport](opts); !ok { | ||
| opts = append(opts, opt.OnFailure(fmt.Sprintf("expected %v, got nil", expected))) | ||
| } | ||
| Fail(opts...) |
There was a problem hiding this comment.
Critical: Expectation context not preserved when calling Fail.
Three issues here:
- Required flag ignored: If
e.requiredis true (e.g., fromRequire(value).Is(nil)), callingFail(opts...)won't abort the test becauseopt.IsRequired(true)isn't added toopts. - Expectation name lost: The default error message (line 316) doesn't include
e.name, breaking the user's expectation labeling. - Wrong test context:
FailcallsT()instead of usinge.t, which can report to the wrongTestingTin parallel tests or subtests.
Apply this diff to propagate the expectation's context:
case any(expected) != nil && any(e.subject) == nil:
if _, ok := opt.Get[opt.FailureReport](opts); !ok {
- opts = append(opts, opt.OnFailure(fmt.Sprintf("expected %v, got nil", expected)))
+ msg := fmt.Sprintf("expected %v, got nil", expected)
+ opts = append(opts, opt.FailureReport(func(...any) []string {
+ return []string{msg}
+ }))
}
+ if e.required {
+ opts = append(opts, opt.IsRequired(true))
+ }
- Fail(opts...)
+ // Use a simple holder struct so e.fail can extract the expected value
+ type holder struct{ Expected T }
+ e.fail(&holder{Expected: expected}, opts...)This ensures:
e.failrespectse.t,e.name, ande.required- Custom failure reports in
optsare honored - The expected value is available to
defaultFailureReportif needed
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case any(expected) != nil && any(e.subject) == nil: | |
| if _, ok := opt.Get[opt.FailureReport](opts); !ok { | |
| opts = append(opts, opt.OnFailure(fmt.Sprintf("expected %v, got nil", expected))) | |
| } | |
| Fail(opts...) | |
| case any(expected) != nil && any(e.subject) == nil: | |
| if _, ok := opt.Get[opt.FailureReport](opts); !ok { | |
| msg := fmt.Sprintf("expected %v, got nil", expected) | |
| opts = append(opts, opt.FailureReport(func(...any) []string { | |
| return []string{msg} | |
| })) | |
| } | |
| if e.required { | |
| opts = append(opts, opt.IsRequired(true)) | |
| } | |
| // Use a simple holder struct so e.fail can extract the expected value | |
| type holder struct{ Expected T } | |
| e.fail(&holder{Expected: expected}, opts...) |
🤖 Prompt for AI Agents
In expectation.go around lines 314–318, the failure call currently drops the
expectation context: it should use the expectation's TestingT (e.t) instead of
T(), include e.name in the failure context, honor e.required by adding
opt.IsRequired(true) when missing, preserve any custom FailureReport in opts,
and ensure the expected value is available to defaultFailureReport by appending
opt.Expected(expected) when not already present; update the Fail/e.fail
invocation to pass e.t, e.name and the augmented opts so e.fail respects e.t,
e.name, e.required, and custom reports.
| type T interface { | ||
| Helper() | ||
| Run(string, func(*testing.T)) bool | ||
| } | ||
| t := testframe.MustPeek[T]() | ||
| t.Helper() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Rename local interface for clarity
Using the name T here can be confused with test.T(). Use a more descriptive name.
Apply this diff:
- type T interface {
+ type tRunner interface {
Helper()
Run(string, func(*testing.T)) bool
}
- t := testframe.MustPeek[T]()
+ t := testframe.MustPeek[tRunner]()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type T interface { | |
| Helper() | |
| Run(string, func(*testing.T)) bool | |
| } | |
| t := testframe.MustPeek[T]() | |
| t.Helper() | |
| type tRunner interface { | |
| Helper() | |
| Run(string, func(*testing.T)) bool | |
| } | |
| t := testframe.MustPeek[tRunner]() | |
| t.Helper() |
🤖 Prompt for AI Agents
In runnable-test.go around lines 28 to 33, the local interface is named T which
is easily confused with testing.T; rename the interface to a more descriptive
name (e.g., TestRunner or TestFrame) and update the MustPeek generic usage and
the variable declaration accordingly (e.g., testframe.MustPeek[TestRunner]() ->
testframe.MustPeek[TestRunner]() and change t to a clearer name like tr),
preserving the call to Helper() and any Run(...) usage; ensure all references in
this scope are updated to the new type and variable name.
| func Error(err error, msg ...string) { | ||
| if s := strings.Join(msg, "\n"); len(s) > 0 { | ||
| err = fmt.Errorf("%w\n%s", err, s) | ||
| } | ||
|
|
||
| if t, ok := testframe.Peek[runner](); ok { | ||
| t.Helper() | ||
| t.Errorf("<== INVALID TEST\nERROR: %s", err.Error()) | ||
| t.FailNow() | ||
| return | ||
| } | ||
|
|
||
| panic(fmt.Errorf("INVALID TEST\n%w", err)) | ||
| } |
There was a problem hiding this comment.
Guard against nil error.
If Error(nil) is called without messages, err remains nil and line 50's err.Error() will panic with a nil pointer dereference.
Apply this diff to handle nil errors:
func Error(err error, msg ...string) {
+ if err == nil {
+ Invalid(append([]string{"Error called with nil error"}, msg...)...)
+ return
+ }
if s := strings.Join(msg, "\n"); len(s) > 0 {
err = fmt.Errorf("%w\n%s", err, s)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func Error(err error, msg ...string) { | |
| if s := strings.Join(msg, "\n"); len(s) > 0 { | |
| err = fmt.Errorf("%w\n%s", err, s) | |
| } | |
| if t, ok := testframe.Peek[runner](); ok { | |
| t.Helper() | |
| t.Errorf("<== INVALID TEST\nERROR: %s", err.Error()) | |
| t.FailNow() | |
| return | |
| } | |
| panic(fmt.Errorf("INVALID TEST\n%w", err)) | |
| } | |
| func Error(err error, msg ...string) { | |
| if err == nil { | |
| Invalid(append([]string{"Error called with nil error"}, msg...)...) | |
| return | |
| } | |
| if s := strings.Join(msg, "\n"); len(s) > 0 { | |
| err = fmt.Errorf("%w\n%s", err, s) | |
| } | |
| if t, ok := testframe.Peek[runner](); ok { | |
| t.Helper() | |
| t.Errorf("<== INVALID TEST\nERROR: %s", err.Error()) | |
| t.FailNow() | |
| return | |
| } | |
| panic(fmt.Errorf("INVALID TEST\n%w", err)) | |
| } |
🤖 Prompt for AI Agents
In test/invalidate.go around lines 43 to 56, the function can panic if
Error(nil) is called because err is nil and err.Error() is invoked; guard
against a nil error by checking if err == nil and replacing it with a non-nil
error (e.g., err = fmt.Errorf("nil error")) before any use of err.Error() or
before wrapping it with fmt.Errorf; ensure this nil-check happens before
joining/wrapping so subsequent logging and the panic use a valid error value.
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
matchers/nilness/matcher.go (1)
63-85: NOT‑nil failure messaging change looks correct; consider typed‑nil consistencyThe new early return for
ToNotMatch(true)whensubject == nilcleanly aligns the message with the updated test ("expected not nil") and avoids the previous"got nil"suffix. That’s a targeted, behaviorally clear tweak.One minor consideration: this short‑circuit only triggers for interface‑nil values (
subject == nil). Typed nils held in an interface (e.g.[]int(nil),*T(nil)) will still go through the existingreflectpath and produce the older, more detailed formatting. If you’d prefer all nil values in NOT‑nil failures to share the simplified"expected not nil"message, you could broaden the check to reuse the existingisNilhelper here rather than onlysubject == nil. Based on learnings, this would stay consistent with how nilability is already determined elsewhere.
♻️ Duplicate comments (19)
matchers/panics/match-recovered_test.go (1)
33-33: Correct usage of outcome constant, but consistency issue persists.The usage of
test.Panickedas the first argument toresult.Expect()is correct and aligns with the restructured test framework. However, other panic test scenarios in this file (e.g., lines 36-53, 212-224) still only validate error messages without asserting the outcome type.The consistency concern was previously raised: consider adding explicit
test.Panickedassertions to other panic test cases for uniformity.match-emptiness_test.go (2)
13-19: Remove redundant slice literal wrapper.The slice literal wrapper with spread operator is unnecessary.
HelperTestsaccepts variadicHelperScenarioarguments, so the scenario can be passed directly.Apply this diff to simplify:
- Run(HelperTests([]HelperScenario{ - {Scenario: "empty slice is empty", + Run(HelperTests( + HelperScenario{Scenario: "empty slice is empty", Act: func() { Expect([]int{}).Should(BeEmpty()) }, }, - }...)) + ))
25-32: Remove redundant slice literal wrapper.The slice literal wrapper with spread operator is unnecessary. Pass the scenario directly as a variadic argument to
HelperTests.Apply this diff to simplify:
- Run(HelperTests([]HelperScenario{ - {Scenario: "nil slice is empty", + Run(HelperTests( + HelperScenario{Scenario: "nil slice is empty", Act: func() { var nilSlice []int Expect(nilSlice).Should(BeEmptyOrNil()) }, }, - }...)) + ))match-context_test.go (1)
20-34: SimplifyHelperTestscalls by removing redundant slice literal.
HelperTestsis variadic, so wrapping scenarios in[]HelperScenario{ ... }...is unnecessary; you can pass them directly for cleaner code. Same change applies to bothContextKeyandContextValueblocks.- Run(HelperTests([]HelperScenario{ + Run(HelperTests( {Scenario: "expected key present", Act: func() { Expect(ctx).To(HaveContextKey(key("key"))) }, }, {Scenario: "expected key not present", Act: func() { Expect(ctx).To(HaveContextKey(key("other-key"))) }, Assert: func(result *R) { result.Expect(test.Failed, opt.IgnoreReport(true)) }, }, - }...)) + )) @@ - Run(HelperTests([]HelperScenario{ + Run(HelperTests( {Scenario: "expected value present", Act: func() { Expect(ctx).To(HaveContextValue(key("key"), "value")) }, }, @@ Assert: func(result *R) { result.Expect(test.Failed, opt.IgnoreReport(true)) }, }, - }...)) + ))Also applies to: 38-60
matchers/typecheck/matcher.go (1)
36-37: LGTM! Cleaner type name derivation.The refactor to use
internal.TypeName[T]()simplifies the code by replacing the reflect-based approach with a centralized utility.matchers/typecheck/match-error.go (2)
33-35: Fix unused parameter in Match signatureThe
optsparameter is declared but never used. Go requires renaming it to_to indicate intentionally unused.Apply this diff:
-func (m MatchError[E]) Match(got error, opts ...any) bool { +func (m MatchError[E]) Match(got error, _ ...any) bool { return errors.As(got, &m.Target) }
58-64: Update comment to match actual behaviorThe comment states "the type of got is not reported when not-nil", but the code at line 63 explicitly includes
internal.TypeName(got)in the report.Apply this diff to align the comment with the code:
- // the type of got is not reported when not-nil since the desired - // error of type E may be in an expected wrapped error; the type of - // got is not particularly helpful + // the type of got is included even when non-nil for debugging context. + // While the expected error of type E may be wrapped, seeing the outer + // error type can still be useful return []string{ "expected: " + expectedType, "got : " + internal.TypeName(got), }expectation.go (5)
313-318: Critical: Expectation context not preserved when callingFail.When the subject is nil but expected is not, the code calls
Fail(opts...)which loses the expectation's context:
e.requiredis not propagated to abort the test on failuree.nameis not included in the failure messageFailusesT()instead ofe.tBased on learnings, the expectation's test instance (
e.t) and name should be preserved. Consider usinge.fail()or properly propagating all context.
322-334: Critical: Expectation context not preserved when callingFailIfNot.Same issues as lines 313-318:
e.required,e.name, ande.tare not propagated when callingFailIfNot(errors.Is(goterr, experr), opts...).
336-337: Critical: Required flag not propagated to delegated expectation.When
e.requiredis true (fromRequire(value).Is(expected)), the new expectation created here won't be required unlessopt.IsRequired(true)is inopts.
389-399: Critical: Expectation context not preserved when callingFailIf.Same issues as in the
Ismethod:e.required,e.name, ande.tare not propagated to theFailIfcall.
402-403: Critical: Required flag not propagated to delegated expectation.Same issue as line 336-337: when
e.requiredis true, the new expectation won't inherit this requirement.match-errors-nil-and-panics.go (1)
284-293: Clarify NilPanic vs Panic(nil) distinction.The comment states "It is equivalent to Panic(nil)" but this is misleading.
Panic(nil)signals "no panic expected" (line 317), whileNilPanic()matches an actualpanic(nil). The documentation should clearly distinguish these cases.test/invalidate.go (3)
27-28: Clarify or remove confusing self-reference.The documentation references
[github.com/blugnu/test.Error], which appears to be this function itself. Either remove this self-reference or clarify what otherErrorfunction you're referring to.
43-56: Guard against nil error.If
Error(nil)is called without messages,errremains nil and line 50'serr.Error()will panic with a nil pointer dereference.Apply this diff to handle nil errors:
func Error(err error, msg ...string) { + if err == nil { + Invalid(append([]string{"Error called with nil error"}, msg...)...) + return + } if s := strings.Join(msg, "\n"); len(s) > 0 { err = fmt.Errorf("%w\n%s", err, s) }
92-98: Clarify misleading comment about test failure.The comment states "it does fail any current test execution," but the implementation only calls
t.Errorf()(line 115), which marks the test as failed without stopping execution. UnlikeInvalidandErrorwhich callt.FailNow(),Warningallows the test to continue running.Consider revising the comment to clarify:
-// Although a warning does not invalidate a test, it does fail any current -// test execution. +// Although a warning does not invalidate a test, it marks the test as failed +// while allowing execution to continue.runnable-test.go (2)
22-24: Verify usage or unexport Helper interface.The exported
Helperinterface is introduced but its usage across the codebase is unclear. Either confirm it's required by external packages or make it unexported.Run this script to check for external usage:
#!/bin/bash # Search for references to test.Helper type (not method calls) rg -n '\btest\.Helper\b' --type go | grep -v 'test\.Helper()'
28-33: Rename local interface for clarity.Using
Tas the interface name conflicts conceptually with thetest.T()function, making the code harder to read.Apply this diff:
- type T interface { + type testRunner interface { Helper() Run(string, func(*testing.T)) bool } - t := testframe.MustPeek[T]() + t := testframe.MustPeek[testRunner]()test-helper.go (1)
83-133: FixR.Expectbehavior when onlytest.Failedis expected (no report/IgnoreReport)When a caller does
r.Expect(test.Failed)(no expected report, noopt.IgnoreReport(true)),analyseArgssetsexpectedOutcometotest.Failed,assertOutcomepasses if the inner test actually failed, but theswitchfalls through todefaultand callsr.TestPassed(opts...). That then asserts there are no failed tests and an empty report, so the outer test fails even though the inner test correctly failed.You need an explicit branch for
expectedOutcome == test.Failed(withoutIgnoreReport) that at least asserts the current test name appears inr.FailedTests, instead of treating it as a success case. For example:switch { case expectedOutcome == test.Panicked: r.assertPanicked(expectedReport, opts...) case len(expectedReport) > 0: // ... existing report checks ... case expectedOutcome == test.Failed && opt.IsSet(opts, opt.IgnoreReport(true)): Expect(r.FailedTests, cFailedTests).To(ContainItem(r.t.Name()), opts...) + case expectedOutcome == test.Failed: + Expect(r.FailedTests, cFailedTests).To(ContainItem(r.t.Name()), opts...) + default: r.TestPassed(opts...) }This preserves the existing
IgnoreReportbehavior while makingr.Expect(test.Failed)(and variants without explicit report lines) behave as intended.Also applies to: 173-199
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (95)
README.md(21 hunks)bool.go(0 hunks)context_examples_test.go(0 hunks)context_test.go(0 hunks)emptiness_test.go(0 hunks)error.go(0 hunks)error_test.go(0 hunks)errors.go(1 hunks)expect/bool.go(1 hunks)expect/bool_test.go(1 hunks)expect/error.go(1 hunks)expect/error_test.go(1 hunks)expect/nil.go(1 hunks)expect/nil_test.go(1 hunks)expect/pkgdoc.go(1 hunks)expect/type.go(1 hunks)expect/type_test.go(1 hunks)expect_didOccur.go(0 hunks)expect_didOccur_test.go(0 hunks)expectation.go(3 hunks)expectation_test.go(6 hunks)fail.go(1 hunks)fail_test.go(1 hunks)internal/testcase/runner.go(1 hunks)internal/testcase/runner_test.go(2 hunks)internal/testframe/errors.go(1 hunks)internal/testframe/is-parallel.go(1 hunks)internal/testframe/is-parallel_test.go(1 hunks)internal/testframe/stack.go(1 hunks)internal/testframe/stack_test.go(1 hunks)internal/type-name.go(1 hunks)internal/type-name_test.go(1 hunks)match-boolean.go(1 hunks)match-boolean_test.go(7 hunks)match-bytes.go(1 hunks)match-bytes_test.go(2 hunks)match-context_test.go(1 hunks)match-emptiness.go(0 hunks)match-emptiness_test.go(1 hunks)match-equals.go(3 hunks)match-equals_test.go(1 hunks)match-errors-nil-and-panics.go(1 hunks)match-errors-nil-and-panics_test.go(1 hunks)match-length.go(1 hunks)match-length_test.go(1 hunks)match-typecheck.go(1 hunks)match-typecheck_test.go(1 hunks)matchers/bools/bools_test.go(2 hunks)matchers/bools/equal.go(2 hunks)matchers/nilness/matcher.go(1 hunks)matchers/nilness/matcher_test.go(1 hunks)matchers/panics/match-recovered_test.go(2 hunks)matchers/slices/contains-item.go(1 hunks)matchers/slices/contains-slice.go(1 hunks)matchers/slices/equals-slice.go(1 hunks)matchers/typecheck/match-error.go(1 hunks)matchers/typecheck/match-error_test.go(1 hunks)matchers/typecheck/matcher.go(2 hunks)mock-fn.go(1 hunks)nilness.go(0 hunks)nilness_test.go(0 hunks)opt/failure-report.go(1 hunks)opt/get.go(2 hunks)opt/opts.go(1 hunks)opt/opts_test.go(5 hunks)opt/value-as-string.go(1 hunks)panic.go(0 hunks)panic_test.go(0 hunks)parallel.go(2 hunks)parallel_test.go(1 hunks)pkgdocs.go(1 hunks)require/bool.go(1 hunks)require/bool_test.go(1 hunks)require/error.go(1 hunks)require/error_test.go(1 hunks)require/nil.go(1 hunks)require/nil_test.go(1 hunks)require/pkgdoc.go(1 hunks)require/type.go(1 hunks)require/type_test.go(1 hunks)run.go(1 hunks)run_test.go(2 hunks)runnable-flaky.go(5 hunks)runnable-flaky_test.go(7 hunks)runnable-helper-tests.go(1 hunks)runnable-test.go(2 hunks)runnable-testcases.go(2 hunks)test-helper.go(13 hunks)test-helper_test.go(9 hunks)test/invalidate.go(3 hunks)test/outcome.go(1 hunks)test/outcome_test.go(1 hunks)testframe.go(1 hunks)type.go(0 hunks)type_test.go(0 hunks)
💤 Files with no reviewable changes (15)
- emptiness_test.go
- panic.go
- expect_didOccur_test.go
- nilness.go
- bool.go
- match-emptiness.go
- type_test.go
- type.go
- panic_test.go
- expect_didOccur.go
- nilness_test.go
- context_test.go
- error.go
- context_examples_test.go
- error_test.go
🧰 Additional context used
🧠 Learnings (25)
📓 Common learnings
Learnt from: deltics
Repo: blugnu/test PR: 47
File: require/error.go:21-21
Timestamp: 2025-10-04T22:53:15.856Z
Learning: In the `require` and `expect` packages, use `test.T()` instead of `test.GetT()` to retrieve the test instance. The `GetT()` function is specifically provided for generic functions that have a type parameter named `T`, which would shadow the `T()` function from the test package. Since these packages cannot use dot-imports and their generic functions use different type parameter names (e.g., `E` for Error), there is no naming conflict with `T()`.
📚 Learning: 2025-06-15T03:06:27.000Z
Learnt from: deltics
Repo: blugnu/test PR: 30
File: test/example.go:13-14
Timestamp: 2025-06-15T03:06:27.000Z
Learning: The Example() function in test/example.go intentionally pushes testframe.example instances onto the stack without popping them. This is by design because: 1) The frames need to remain available for subsequent example code to use via T(), 2) Doc examples run in a single goroutine per package and are short-lived, 3) The memory overhead is negligible, and 4) The complexity of managing frame cleanup would introduce more risk than benefit.
Applied to files:
internal/testframe/errors.gorun.gomatch-errors-nil-and-panics_test.gointernal/testframe/stack.gomatch-boolean_test.gorunnable-test.gointernal/testframe/stack_test.goexpect/bool_test.goREADME.mdinternal/testframe/is-parallel_test.gotest/invalidate.gomatch-equals_test.gotest/outcome_test.gotestframe.go
📚 Learning: 2025-10-04T22:53:15.856Z
Learnt from: deltics
Repo: blugnu/test PR: 47
File: require/error.go:21-21
Timestamp: 2025-10-04T22:53:15.856Z
Learning: In the `require` and `expect` packages, use `test.T()` instead of `test.GetT()` to retrieve the test instance. The `GetT()` function is specifically provided for generic functions that have a type parameter named `T`, which would shadow the `T()` function from the test package. Since these packages cannot use dot-imports and their generic functions use different type parameter names (e.g., `E` for Error), there is no naming conflict with `T()`.
Applied to files:
internal/testcase/runner_test.gorequire/pkgdoc.gorequire/error.goexpect/bool.gorequire/bool.gomatch-emptiness_test.gorequire/type.gorequire/error_test.gomatch-typecheck.gorequire/nil.goexpect/nil.gomatchers/typecheck/match-error_test.goexpect/type.gotest/outcome.gomatchers/typecheck/match-error.goexpect/pkgdoc.gomatchers/typecheck/matcher.goopt/opts_test.goexpect/type_test.goexpect/nil_test.gomatch-errors-nil-and-panics_test.goexpect/error.gorunnable-flaky_test.gorequire/type_test.gomatch-boolean_test.gofail.gorunnable-test.gorequire/nil_test.gorequire/bool_test.gointernal/type-name_test.goexpect/error_test.gomatchers/panics/match-recovered_test.goexpectation_test.goexpect/bool_test.goREADME.mdinternal/testframe/is-parallel_test.gopkgdocs.gotest-helper_test.goexpectation.gofail_test.gomatch-equals_test.gorunnable-flaky.gomatch-context_test.gotest/outcome_test.goparallel_test.gomatch-typecheck_test.gotest-helper.gorun_test.go
📚 Learning: 2025-06-13T22:48:59.724Z
Learnt from: deltics
Repo: blugnu/test PR: 30
File: parallel.go:3-8
Timestamp: 2025-06-13T22:48:59.724Z
Learning: In Go, importing a package with the same name as the current package (e.g., importing "github.com/blugnu/test/test" in package "test") does not cause confusion because local symbols are always unqualified while imported symbols are always qualified with the package name. There's no possibility of accidental self-references since you cannot mistakenly call test.LocalFunction() when you mean LocalFunction() or vice versa.
Applied to files:
internal/testcase/runner_test.goopt/opts_test.gorunnable-flaky_test.gomatch-boolean_test.gointernal/type-name_test.gomatchers/panics/match-recovered_test.goREADME.mdtest/invalidate.go
📚 Learning: 2025-06-13T23:21:32.748Z
Learnt from: deltics
Repo: blugnu/test PR: 30
File: type.go:7-9
Timestamp: 2025-06-13T23:21:32.748Z
Learning: In Go, a package can import a subpackage with the same name without creating an import cycle. For example, package `test` can import `"github.com/blugnu/test/test"` (a subpackage) without issues. Import cycles only occur when packages import each other directly or indirectly.
Applied to files:
internal/testcase/runner_test.goopt/opts_test.gomatch-boolean_test.go
📚 Learning: 2025-06-14T00:32:58.151Z
Learnt from: deltics
Repo: blugnu/test PR: 30
File: t.go:84-84
Timestamp: 2025-06-14T00:32:58.151Z
Learning: ExampleTestRunner in the blugnu/test package is intentionally a minimal implementation designed solely for runnable examples in package documentation, not for complete testing functionality. Frame management and cleanup completeness are not priorities for this use case.
Applied to files:
internal/testcase/runner_test.goREADME.mdinternal/testframe/is-parallel_test.gotest/invalidate.gotest/outcome_test.go
📚 Learning: 2025-06-15T03:58:31.152Z
Learnt from: deltics
Repo: blugnu/test PR: 30
File: README.md:0-0
Timestamp: 2025-06-15T03:58:31.152Z
Learning: When documenting functions in the blugnu/test package, prioritize showing typical usage patterns over technical function signatures. For example, With(t *testing.T) should be documented as accepting *testing.T even though it technically accepts the TestingT interface, because that's how users will actually call it.
Applied to files:
internal/testcase/runner_test.gorequire/pkgdoc.gorequire/error.goexpect/bool.gorequire/bool.gomatch-emptiness_test.gorequire/type.gorequire/error_test.gomatch-typecheck.gomatchers/typecheck/match-error_test.goexpect/type.gomatchers/typecheck/matcher.goopt/opts_test.goexpect/type_test.goexpect/nil_test.gomatch-errors-nil-and-panics_test.gorunnable-flaky_test.gorequire/type_test.gomatch-boolean_test.gofail.gorunnable-test.gorunnable-testcases.gointernal/testframe/stack_test.gorequire/bool_test.gointernal/type-name_test.goexpect/error_test.gomatchers/panics/match-recovered_test.goexpectation_test.goREADME.mdtest-helper_test.gofail_test.gomatch-length_test.gomatch-equals_test.gomatch-context_test.gotest/outcome_test.gotestframe.gomatch-typecheck_test.go
📚 Learning: 2025-05-30T18:55:32.543Z
Learnt from: deltics
Repo: blugnu/test PR: 25
File: expect_test.go:68-68
Timestamp: 2025-05-30T18:55:32.543Z
Learning: The `R.Expect` method in the `test` package automatically verifies the presence of the test filename in failure reports, eliminating the need for explicit `TestFilename()` calls in test assertions and simplifying both single-line and multi-line report expectations.
Applied to files:
internal/testcase/runner_test.gorequire/pkgdoc.gorequire/error.gomock-fn.goexpect/bool.gomatchers/bools/equal.gorequire/error_test.goexpect/nil.goexpect/type.goexpect/pkgdoc.goopt/opts_test.goexpect/type_test.goexpect/nil_test.gomatch-errors-nil-and-panics_test.goexpect/error.gorunnable-flaky_test.gorequire/type_test.gomatch-boolean_test.gofail.gomatch-errors-nil-and-panics.gorunnable-helper-tests.gorequire/bool_test.goexpect/error_test.gomatchers/panics/match-recovered_test.goexpectation_test.goexpect/bool_test.goREADME.mdtest-helper_test.goexpectation.gofail_test.gomatchers/bools/bools_test.gotest/outcome_test.gotest-helper.gorun_test.go
📚 Learning: 2025-06-14T00:35:50.260Z
Learnt from: deltics
Repo: blugnu/test PR: 30
File: t_test.go:45-52
Timestamp: 2025-06-14T00:35:50.260Z
Learning: In the blugnu/test framework, `defer Expect(Panic(...)).DidOccur()` works by executing `Expect(Panic(...))` immediately to set up the panic expectation, while only deferring the `.DidOccur()` method call for verification after the panic occurs. This allows proper panic testing where the expectation is established before the panic and verification happens after.
Applied to files:
internal/testcase/runner_test.gomatch-errors-nil-and-panics_test.gorunnable-flaky_test.gomatch-errors-nil-and-panics.gomatchers/panics/match-recovered_test.goexpectation_test.goexpect/bool_test.goREADME.mdtest-helper_test.gotest-helper.go
📚 Learning: 2025-05-27T08:15:10.945Z
Learnt from: deltics
Repo: blugnu/test PR: 25
File: parallel.go:17-20
Timestamp: 2025-05-27T08:15:10.945Z
Learning: In the `isParallel` function in parallel.go, the check `if _, ok := T().(*testing.T); !ok` is intentional and checks the global test frame T() rather than the parameter t. This serves as a precondition - if T() is not a *testing.T, then parallel execution is not supported by the testing framework context, making it unnecessary to check the parallel state of the specific test instance parameter t.
Applied to files:
internal/testframe/is-parallel.gorequire/error.gorunnable-flaky_test.goparallel.gorunnable-test.gomatchers/panics/match-recovered_test.goexpectation_test.goREADME.mdinternal/testframe/is-parallel_test.gotest/invalidate.gotest-helper_test.gotest/outcome_test.goparallel_test.gotestframe.go
📚 Learning: 2024-11-08T01:07:59.462Z
Learnt from: deltics
Repo: blugnu/test PR: 23
File: mockFn.go:236-283
Timestamp: 2024-11-08T01:07:59.462Z
Learning: In the `mockFn.go` file for the `MockFn` type in the `test` package, mocks are not intended to be used in concurrent tests, and making the mock functions thread-safe is inappropriate. Test authors must ensure that each test uses a separate and distinct mock when running concurrent tests.
Applied to files:
internal/testframe/is-parallel.gomock-fn.gorunnable-test.gointernal/testframe/stack_test.goREADME.mdinternal/testframe/is-parallel_test.goparallel_test.go
📚 Learning: 2025-06-26T07:45:31.021Z
Learnt from: deltics
Repo: blugnu/test PR: 31
File: matchers/slices/containsItem.go:22-26
Timestamp: 2025-06-26T07:45:31.021Z
Learning: In the blugnu/test codebase, the comma/ok idiom for type assertions (e.g., `at, _ := a.(T)`) is used to satisfy linting requirements that complain about unchecked assertions, even when the type constraints guarantee the assertions will always succeed. The ok result is deliberately ignored because the calling code ensures type safety, making this a consistent pattern throughout the codebase.
Applied to files:
require/pkgdoc.gorequire/error.gorequire/type.gomatchers/typecheck/match-error_test.gomatchers/typecheck/match-error.goexpect/pkgdoc.gomatchers/typecheck/matcher.goopt/opts_test.goexpect/type_test.goexpect/nil_test.gomatch-errors-nil-and-panics_test.gorequire/type_test.gorequire/nil_test.gomatchers/panics/match-recovered_test.goexpect/bool_test.goREADME.mdtest-helper_test.gotest/outcome_test.gomatch-typecheck_test.go
📚 Learning: 2025-05-30T18:15:33.258Z
Learnt from: deltics
Repo: blugnu/test PR: 25
File: expect_isEmpty.go:230-236
Timestamp: 2025-05-30T18:15:33.258Z
Learning: The e.err() method in the expectation[T] type accepts an 'any' parameter with special handling for nil, string, or []string values. Single-line error messages use string while multi-line reports use []string, allowing for different formatting approaches in test failure reporting.
Applied to files:
require/error.gomatchers/nilness/matcher_test.gorequire/error_test.goexpect/nil.gomatchers/typecheck/match-error_test.gomatchers/typecheck/match-error.goexpect/pkgdoc.gomatchers/typecheck/matcher.goopt/opts_test.goexpect/type_test.goexpect/nil_test.gomatch-errors-nil-and-panics_test.goexpect/error.gomatch-boolean_test.gofail.gomatch-errors-nil-and-panics.gorequire/nil_test.goexpect/error_test.gomatchers/panics/match-recovered_test.goexpectation_test.goexpect/bool_test.goREADME.mdtest/invalidate.gotest-helper_test.goexpectation.gofail_test.gomatchers/slices/equals-slice.gotest-helper.go
📚 Learning: 2025-06-15T03:06:27.000Z
Learnt from: deltics
Repo: blugnu/test PR: 30
File: test/example.go:13-14
Timestamp: 2025-06-15T03:06:27.000Z
Learning: The T() function returns the most recent frame from the current testframe stack, so accumulating frames in Example() functions ensures the correct frame is available for example code execution.
Applied to files:
require/error.goREADME.mdtestframe.go
📚 Learning: 2025-06-15T01:58:08.566Z
Learnt from: deltics
Repo: blugnu/test PR: 30
File: matchers/emptiness/matcher.go:45-50
Timestamp: 2025-06-15T01:58:08.566Z
Learning: In the emptiness.Matcher implementation, the TreatNilAsEmpty flag is applied within the hasLen() method. For typed nil values (nil slices, maps, channels), m.isNil is set to !m.TreatNilAsEmpty. If m.isNil is true after hasLen() executes, it means nil should not be treated as empty in that context. Untyped nil is never considered empty regardless of the TreatNilAsEmpty flag.
Applied to files:
matchers/nilness/matcher_test.gomatch-emptiness_test.goexpect/nil.goexpect/nil_test.gomatchers/nilness/matcher.gomatch-length_test.go
📚 Learning: 2025-06-15T03:37:45.516Z
Learnt from: deltics
Repo: blugnu/test PR: 30
File: runScenarios.go:60-66
Timestamp: 2025-06-15T03:37:45.516Z
Learning: In Go reflection, when calling reflect.ValueOf(nil), it returns an invalid Value, and calling Kind() on an invalid Value returns reflect.Invalid (not reflect.Struct). This means that code checking ref.Kind() != reflect.Struct will handle nil values correctly without panicking by returning early, as seen in the getScenarioName function in runScenarios.go.
Applied to files:
matchers/nilness/matcher_test.gorequire/nil.goexpect/nil.goexpect/nil_test.gomatch-errors-nil-and-panics_test.gomatch-errors-nil-and-panics.gorequire/nil_test.gotest/invalidate.gomatchers/nilness/matcher.go
📚 Learning: 2024-11-07T23:40:52.974Z
Learnt from: deltics
Repo: blugnu/test PR: 23
File: mockFn_test.go:194-232
Timestamp: 2024-11-07T23:40:52.974Z
Learning: Calling `ExpectCall()` after a `Reset()` is functionally equivalent to the first call to `ExpectCall()` on a new `MockFn()`, which is already covered by existing tests.
Applied to files:
mock-fn.goexpectation_test.goREADME.mdtest-helper.go
📚 Learning: 2025-05-27T08:12:32.068Z
Learnt from: deltics
Repo: blugnu/test PR: 25
File: ordered_test.go:8-20
Timestamp: 2025-05-27T08:12:32.068Z
Learning: In the test package, factory function tests (like BeGreaterThan, BeLessThan) are kept separate from matcher implementation tests. Factory function tests provide coverage for the factory functions themselves, while edge cases and comprehensive logic testing are covered in the tests for the matcher implementation itself.
Applied to files:
match-emptiness_test.gomatch-typecheck.goREADME.mdmatch-length_test.go
📚 Learning: 2025-05-30T19:01:33.695Z
Learnt from: deltics
Repo: blugnu/test PR: 25
File: matchers/contexts/valueMatcher.go:21-21
Timestamp: 2025-05-30T19:01:33.695Z
Learning: In heterogeneous matchers, when `MatchAny` is called by the framework, the type of the `got` parameter is guaranteed to match the type parameter `T` because the framework has already verified type compatibility through the `Match` method. This makes type assertions in `MatchAny` implementations safe.
Applied to files:
require/type.gomatch-typecheck.goexpect/type.gomatchers/typecheck/matcher.goREADME.mdpkgdocs.goexpectation.go
📚 Learning: 2025-05-27T07:47:57.910Z
Learnt from: deltics
Repo: blugnu/test PR: 25
File: matchers/maps/equal.go:11-17
Timestamp: 2025-05-27T07:47:57.910Z
Learning: In matchers/maps/utils.go, the compareFuncFor function includes a fallback mechanism that defaults to reflect.DeepEqual when no other suitable comparator is found from type interfaces, custom functions, or special cases.
Applied to files:
match-typecheck.gomatchers/typecheck/matcher.goexpectation_test.goREADME.mdpkgdocs.goexpectation.gomatch-equals_test.gomatch-equals.go
📚 Learning: 2025-06-13T22:45:11.744Z
Learnt from: deltics
Repo: blugnu/test PR: 30
File: opt/valueAsString.go:24-34
Timestamp: 2025-06-13T22:45:11.744Z
Learning: In opt/valueAsString.go, the ValueAsString function deliberately applies the AsDeclaration option only to non-string values. String values are formatted as strings and are subject only to the QuotedStrings option. The switch statement order is intentional - string cases come before AsDeclaration to ensure strings are never formatted with %#v even when AsDeclaration(true) is set.
Applied to files:
opt/opts_test.goopt/value-as-string.go
📚 Learning: 2025-07-20T01:53:48.920Z
Learnt from: deltics
Repo: blugnu/test PR: 40
File: run-flaky_test.go:10-102
Timestamp: 2025-07-20T01:53:48.920Z
Learning: In the blugnu/test package flaky test runner, the attempt counting logic correctly counts attempts (not retries). If a test runs once and fails, that's 1 attempt. If it runs 3 times total, that's 3 attempts. This is the intended behavior and should not be considered an off-by-one error.
Applied to files:
runnable-flaky_test.goREADME.mdrunnable-flaky.go
📚 Learning: 2025-06-15T02:04:30.372Z
Learnt from: deltics
Repo: blugnu/test PR: 30
File: restore.go:13-23
Timestamp: 2025-06-15T02:04:30.372Z
Learning: In restore.go, the user prefers handling nil pointer guards in the Original() function rather than in ReplacedBy() method, as it's cleaner to catch nil pointers at the entry point rather than at usage.
Applied to files:
match-errors-nil-and-panics.go
📚 Learning: 2025-05-30T18:57:57.344Z
Learnt from: deltics
Repo: blugnu/test PR: 25
File: expect_test.go:593-601
Timestamp: 2025-05-30T18:57:57.344Z
Learning: In heterogenous matchers, the `Match()` method should always return `true` by convention to signal compatibility with the typed parameter `T`. The actual matching logic is implemented in `MatchAny()`, which will be used in preference when present. This design separates type compatibility signaling from actual matching behavior.
Applied to files:
README.mdpkgdocs.go
📚 Learning: 2024-11-07T23:05:45.944Z
Learnt from: deltics
Repo: blugnu/test PR: 23
File: context_test.go:12-17
Timestamp: 2024-11-07T23:05:45.944Z
Learning: The function `ContextKey` in `context_test.go` is a test fixture used only with known string values, so the unsafe type assertion `v.(string)` is acceptable.
Applied to files:
README.mdmatch-context_test.go
🧬 Code graph analysis (50)
matchers/slices/contains-slice.go (6)
matchers/strings/regex.go (1)
OnTestFailure(17-29)matchers/equal/equal.go (1)
OnTestFailure(25-27)matchers/maps/contains.go (1)
OnTestFailure(19-32)matchers/bytes/equal.go (1)
OnTestFailure(43-80)matchers/slices/containsItems.go (1)
OnTestFailure(35-44)matchers/ordered/between.go (1)
OnTestFailure(59-85)
internal/testcase/runner_test.go (1)
test/outcome.go (1)
Passed(8-8)
internal/testframe/is-parallel.go (3)
parallel.go (1)
IsParallel(10-12)internal/testframe/stack.go (1)
Peek(68-87)internal/testframe/nilFrame.go (1)
Parallel(45-45)
matchers/slices/contains-item.go (2)
matchers/slices/containsItem.go (2)
Match(19-45)OnTestFailure(47-66)slices.go (1)
ContainItem(24-26)
require/pkgdoc.go (3)
expect_test.go (2)
ExampleRequire(462-474)TestRequire(451-460)type_test.go (1)
TestRequireType(73-96)type.go (1)
RequireType(71-77)
require/error.go (4)
opt/opts.go (1)
Required(77-79)error.go (1)
Error(15-18)expect.go (1)
Require(88-97)type.go (1)
RequireType(71-77)
matchers/nilness/matcher_test.go (1)
nilness_test.go (1)
TestExpect_IsNotNil(49-78)
expect/bool.go (2)
bool.go (2)
ExpectFalse(41-44)ExpectTrue(62-65)bool_test.go (5)
TestBooleans(11-137)ExampleExpectTrue(148-155)ExampleExpectFalse(139-146)ExampleBeTrue(166-173)ExampleBeFalse(157-164)
match-bytes_test.go (2)
bytes_test.go (1)
ExampleEqualBytes(46-60)matchers/bytes/equal.go (1)
OnTestFailure(43-80)
require/bool.go (2)
expectation.go (1)
Require(50-59)match-boolean.go (2)
BeFalse(13-15)BeTrue(23-25)
match-emptiness_test.go (4)
runnable-test.go (1)
T(28-31)run.go (1)
Run(19-27)match-emptiness.go (2)
BeEmpty(40-42)BeEmptyOrNil(73-75)test/example.go (1)
Example(12-29)
match-typecheck.go (1)
type.go (1)
BeOfType(85-87)
expect/nil.go (2)
expectation.go (1)
Expect(22-31)match-errors-nil-and-panics.go (1)
BeNil(278-280)
matchers/typecheck/match-error_test.go (1)
match-errors-nil-and-panics.go (1)
BeError(195-197)
expect/type.go (4)
internal/type-name.go (1)
TypeName(15-48)opt/failure-report.go (1)
FailureReport(13-13)expectation.go (1)
Expect(22-31)match-equals.go (1)
Equal(61-63)
matchers/typecheck/match-error.go (3)
internal/type-name.go (1)
TypeName(15-48)opt/is-set.go (1)
IsSet(3-10)opt/opts.go (1)
ToNotMatch(64-64)
matchers/typecheck/matcher.go (1)
internal/type-name.go (1)
TypeName(15-48)
opt/opts_test.go (3)
expect/type.go (1)
Type(24-51)opt/opts.go (4)
ExactOrder(18-18)IsRequired(32-32)StackTrace(56-56)QuotedStrings(49-49)matchers/panics/stacktrace.go (1)
StackTrace(9-33)
expect/type_test.go (1)
expect/type.go (1)
Type(24-51)
expect/nil_test.go (2)
test/example.go (1)
Example(12-29)expect/nil.go (2)
Nil(21-24)NotNil(42-45)
internal/testframe/stack.go (2)
internal/testframe/nilFrame.go (1)
Cleanup(6-8)internal/testframe/errors.go (1)
ErrNoCleanupFunction(8-8)
expect/error.go (4)
opt/get.go (1)
Get(10-19)opt/failure-report.go (1)
FailureReport(13-13)internal/type-name.go (1)
TypeName(15-48)fail.go (1)
Fail(37-40)
runnable-flaky_test.go (2)
expectation.go (1)
Expect(22-31)test/outcome.go (1)
Passed(8-8)
require/type_test.go (4)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)require/type.go (1)
Type(15-21)
match-boolean_test.go (5)
test-helper.go (1)
R(26-49)expectation.go (1)
Expect(22-31)opt/failure-report.go (1)
FailureReport(13-13)test/example.go (1)
Example(12-29)match-boolean.go (1)
BeFalse(13-15)
fail.go (4)
opt/failure-report.go (2)
OnFailure(21-25)FailureReport(13-13)opt/get.go (1)
Get(10-19)expectation.go (1)
Expect(22-31)match-boolean.go (1)
BeFalse(13-15)
runnable-test.go (3)
run.go (1)
Run(19-27)internal/testframe/stack.go (2)
MustPeek(54-59)PushWithCleanup(134-142)test/invalidate.go (1)
Invalid(71-86)
runnable-testcases.go (2)
runnable-test.go (1)
T(28-31)internal/testcase/runner.go (3)
Registration(53-53)Runner(46-51)Flags(11-11)
match-errors-nil-and-panics.go (6)
matchers/panics/match-recovered.go (2)
Expected(53-55)MatchRecovered(41-51)test-helper.go (1)
R(26-49)test/invalidate.go (1)
Invalid(71-86)opt/opts.go (2)
NoPanicExpected(36-36)ToNotMatch(64-64)opt/failure-report.go (1)
FailureReport(13-13)opt/value-as-string.go (1)
ValueAsString(20-38)
require/nil_test.go (2)
test/example.go (1)
Example(12-29)require/nil.go (2)
Nil(21-24)NotNil(42-45)
match-boolean.go (1)
matchers/bools/equal.go (1)
BooleanMatcher(11-13)
internal/testframe/stack_test.go (3)
internal/testframe/nilFrame.go (1)
Cleanup(6-8)internal/testframe/stack.go (1)
PushWithCleanup(134-142)internal/testframe/errors.go (1)
ErrNoCleanupFunction(8-8)
require/bool_test.go (2)
test/example.go (1)
Example(12-29)require/bool.go (2)
False(21-24)True(44-47)
internal/type-name_test.go (5)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)internal/type-name.go (1)
TypeName(15-48)match-equals.go (1)
Equal(61-63)
matchers/panics/match-recovered_test.go (2)
expectation.go (1)
Expect(22-31)test/outcome.go (1)
Panicked(10-10)
expectation_test.go (7)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(22-31)errors.go (2)
ErrInvalidArgument(10-10)ErrInvalidOperation(13-13)test-helper.go (1)
R(26-49)test/outcome.go (1)
Failed(9-9)
expect/bool_test.go (2)
test/example.go (1)
Example(12-29)expect/bool.go (2)
False(21-24)True(42-45)
internal/testframe/is-parallel_test.go (6)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-test.go (1)
Test(69-71)internal/testframe/is-parallel.go (1)
IsParallel(15-26)parallel.go (2)
IsParallel(10-12)Parallel(37-53)match-boolean.go (2)
BeFalse(13-15)BeTrue(23-25)
test/invalidate.go (2)
fail.go (1)
Errorf(27-30)internal/testframe/stack.go (1)
Peek(68-87)
test-helper_test.go (3)
test/outcome.go (4)
Panicked(10-10)Failed(9-9)Outcome(5-5)Passed(8-8)test-helper.go (1)
R(26-49)testframe.go (1)
T(42-44)
expectation.go (4)
opt/get.go (1)
Get(10-19)opt/failure-report.go (2)
FailureReport(13-13)OnFailure(21-25)fail.go (3)
Fail(37-40)FailIfNot(68-71)FailIf(44-64)match-equals.go (1)
DeepEqual(29-31)
fail_test.go (6)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)fail.go (5)
Error(16-19)Errorf(27-30)Fail(37-40)FailIf(44-64)FailIfNot(68-71)test-helper.go (1)
R(26-49)opt/opts.go (1)
Required(77-79)
match-equals_test.go (6)
testframe.go (1)
With(96-102)run.go (1)
Run(19-27)runnable-helper-tests.go (2)
HelperTests(21-25)HelperScenario(9-15)expectation.go (1)
Expect(22-31)match-equals.go (2)
Equal(61-63)DeepEqual(29-31)test/example.go (1)
Example(12-29)
runnable-flaky.go (1)
test/outcome.go (3)
Passed(8-8)Outcome(5-5)Failed(9-9)
match-context_test.go (2)
match-context.go (2)
HaveContextKey(19-21)HaveContextValue(46-51)test-helper.go (1)
R(26-49)
test/outcome_test.go (1)
test/outcome.go (4)
Outcome(5-5)Passed(8-8)Failed(9-9)Panicked(10-10)
testframe.go (1)
internal/testframe/stack.go (1)
PushWithCleanup(134-142)
test-helper.go (3)
test/outcome.go (4)
Outcome(5-5)Panicked(10-10)Failed(9-9)Passed(8-8)opt/is-set.go (1)
IsSet(3-10)testframe.go (1)
T(42-44)
run_test.go (2)
expectation.go (1)
Expect(22-31)test/outcome.go (1)
Failed(9-9)
match-equals.go (1)
matchers/equal/deepEqual.go (1)
DeepMatcher(12-14)
🪛 GitHub Actions: release
internal/type-name.go
[error] 37-37: golangci-lint: reflect.TypeOf call can be simplified using TypeFor (modernize) in internal/type-name.go:37
🪛 GitHub Check: module-qa / qa / run tests
internal/type-name.go
[failure] 37-37:
reflecttypefor: reflect.TypeOf call can be simplified using TypeFor (modernize)
🪛 LanguageTool
README.md
[style] ~152-~152: This phrase is redundant. Consider using “outside”.
Context: ...However, if a new test frame is created outside of the test package, then With(t) must...
(OUTSIDE_OF)
| {Scenario: "panic(), no panic occurs", | ||
| Act: func() { | ||
| defer Expect(Panic(ErrInvalidArgument)).DidNotOccur() | ||
| }, | ||
| }, | ||
| {Scenario: "panic(r), no panic occurs", | ||
| Act: func() { | ||
| defer Expect(Panic(ErrInvalidArgument)).DidNotOccur() | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove duplicate test scenario.
These two scenarios have identical implementations and effectively the same description. This appears to be a copy-paste error.
Apply this diff to remove the duplicate:
- {Scenario: "panic(), no panic occurs",
- Act: func() {
- defer Expect(Panic(ErrInvalidArgument)).DidNotOccur()
- },
- },
{Scenario: "panic(r), no panic occurs",
Act: func() {
defer Expect(Panic(ErrInvalidArgument)).DidNotOccur()
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {Scenario: "panic(), no panic occurs", | |
| Act: func() { | |
| defer Expect(Panic(ErrInvalidArgument)).DidNotOccur() | |
| }, | |
| }, | |
| {Scenario: "panic(r), no panic occurs", | |
| Act: func() { | |
| defer Expect(Panic(ErrInvalidArgument)).DidNotOccur() | |
| }, | |
| }, | |
| {Scenario: "panic(r), no panic occurs", | |
| Act: func() { | |
| defer Expect(Panic(ErrInvalidArgument)).DidNotOccur() | |
| }, | |
| }, |
🤖 Prompt for AI Agents
In expectation_test.go around lines 346 to 355 there is a duplicated test
scenario ("panic(), no panic occurs" and "panic(r), no panic occurs") with
identical Act implementations; remove one of the two duplicate scenario entries
(either the first or second) so the test suite contains only a single scenario
for the "no panic occurs" case, preserving the remaining entry exactly as-is.
| Scenario: "expecting not nil and got not nil", | ||
| Act: func() { var a any = 1; Expect(a).IsNot(nil) }, | ||
| }, | ||
| { | ||
| Scenario: "expecting not nil when got is of not nilable type", | ||
| Act: func() { var a any = 1; Expect(a).IsNot(nil) }, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove duplicate test scenario.
The scenario at lines 513-515 is a duplicate of lines 509-511. Both test the same condition with the same code.
Apply this diff to remove the duplicate:
{
Scenario: "expecting not nil and got not nil",
Act: func() { var a any = 1; Expect(a).IsNot(nil) },
},
- {
- Scenario: "expecting not nil when got is of not nilable type",
- Act: func() { var a any = 1; Expect(a).IsNot(nil) },
- },🤖 Prompt for AI Agents
In expectation_test.go around lines 509 to 515, there's a duplicate test
scenario: the block with Scenario "expecting not nil when got is of not nilable
type" (lines 513-515) repeats the prior scenario ("expecting not nil and got not
nil"); remove the duplicate block (delete the second scenario lines 513–515) so
only one identical test remains.
| // Error explicitly and unconditionally fails the current test | ||
| // with the given message. | ||
| // | ||
| // This should not be confused with the `test.Error` function | ||
| // used to report an error condition in a test helper (from the | ||
| // blugnu/test/test package). | ||
| func Error(msg string) { | ||
| T().Helper() | ||
| Fail(opt.OnFailure(msg)) | ||
| } | ||
|
|
||
| // Errorf explicitly and unconditionally fails the current test | ||
| // with the formatted message. | ||
| // | ||
| // This should not be confused with the `test.Error` function | ||
| // used to report an error condition in a test helper (from the | ||
| // blugnu/test/test package). | ||
| func Errorf(s string, args ...any) { | ||
| T().Helper() | ||
| Fail(opt.OnFailure(fmt.Sprintf(s, args...))) | ||
| } | ||
|
|
||
| // Fail explicitly and unconditionally fails the current test with the given | ||
| // options. | ||
| // | ||
| // If the supplied options do not contain an [opt.FailureReport], the test is | ||
| // failed with a "test failed" message. | ||
| func Fail(opts ...any) { | ||
| T().Helper() | ||
| FailIf(true, opts...) | ||
| } | ||
|
|
||
| // FailIf fails the current test with the given options only if a specified | ||
| // condition is true. | ||
| func FailIf(cond bool, opts ...any) { | ||
| if !cond { | ||
| return | ||
| } | ||
|
|
||
| T().Helper() | ||
|
|
||
| if _, ok := opt.Get[opt.FailureReport](opts); !ok { | ||
| var msg string = "test failed" | ||
|
|
||
| if s, ok := opt.Get[[]string](opts); ok { | ||
| msg = strings.Join(s, "\n") | ||
| } else if s, ok := opt.Get[string](opts); ok { | ||
| msg = s | ||
| } | ||
|
|
||
| opts = append([]any{opt.OnFailure(msg)}, opts...) | ||
| } | ||
|
|
||
| Expect(cond).To(BeFalse(), opts...) | ||
| } | ||
|
|
||
| // FailIfNot fails the current test with the given options only if a specified | ||
| // condition is not true. | ||
| func FailIfNot(cond bool, opts ...any) { | ||
| T().Helper() | ||
| FailIf(!cond, opts...) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Fail/Error helpers are wired correctly to existing expectation machinery
Error/Errorf → Fail → FailIf → Expect(cond).To(BeFalse(), opts...) gives a clean, consistent path for unconditional failures, and the default "test failed" plus optional string/[]string handling for opt.FailureReport matches the documented behavior. If you ever need richer multi‑line default reports, you could consider constructing an explicit opt.FailureReport from the []string case instead of strings.Join, but the current implementation is functionally sound.
🤖 Prompt for AI Agents
In fail.go around lines 10 to 71, the review confirms Error/Errorf → Fail →
FailIf → Expect(...) are wired correctly and the default "test failed" behavior
plus string/[]string handling for opt.FailureReport is correct; no code changes
are required—leave the implementation as-is (optionally consider in future
constructing an explicit opt.FailureReport for richer multi-line reports instead
of strings.Join).
| func IsParallel() bool { | ||
| t, ok := Peek[*testing.T]() | ||
| if !ok { | ||
| // tests cannot be parallel if the TestingT is not a *testing.T | ||
| return false | ||
| } | ||
|
|
||
| c := reflect.Indirect(reflect.ValueOf(t)).FieldByName("common") | ||
| ip := reflect.Indirect(c).FieldByName("isParallel") | ||
|
|
||
| return ip.Bool() | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding panic protection for reflection operations.
The reflection code accesses unexported fields (common and isParallel) in *testing.T without error checking. If Go's internal structure changes, this could panic at runtime.
Consider adding defensive checks:
func IsParallel() bool {
t, ok := Peek[*testing.T]()
if !ok {
// tests cannot be parallel if the TestingT is not a *testing.T
return false
}
+ defer func() {
+ if recover() != nil {
+ // if reflection fails, assume not parallel
+ }
+ }()
+
c := reflect.Indirect(reflect.ValueOf(t)).FieldByName("common")
+ if !c.IsValid() {
+ return false
+ }
ip := reflect.Indirect(c).FieldByName("isParallel")
+ if !ip.IsValid() || ip.Kind() != reflect.Bool {
+ return false
+ }
return ip.Bool()
}Alternatively, if this pattern was already used and stable in the previous parallel.go implementation, this concern may be acceptable.
Note: This is optional since reflection on stdlib internals is inherently fragile, and the existing pattern may have been tested extensively.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func IsParallel() bool { | |
| t, ok := Peek[*testing.T]() | |
| if !ok { | |
| // tests cannot be parallel if the TestingT is not a *testing.T | |
| return false | |
| } | |
| c := reflect.Indirect(reflect.ValueOf(t)).FieldByName("common") | |
| ip := reflect.Indirect(c).FieldByName("isParallel") | |
| return ip.Bool() | |
| } | |
| func IsParallel() bool { | |
| t, ok := Peek[*testing.T]() | |
| if !ok { | |
| // tests cannot be parallel if the TestingT is not a *testing.T | |
| return false | |
| } | |
| defer func() { | |
| if recover() != nil { | |
| // if reflection fails, assume not parallel | |
| } | |
| }() | |
| c := reflect.Indirect(reflect.ValueOf(t)).FieldByName("common") | |
| if !c.IsValid() { | |
| return false | |
| } | |
| ip := reflect.Indirect(c).FieldByName("isParallel") | |
| if !ip.IsValid() || ip.Kind() != reflect.Bool { | |
| return false | |
| } | |
| return ip.Bool() | |
| } |
🤖 Prompt for AI Agents
internal/testframe/is-parallel.go lines 15-26: the reflection code directly
indexes unexported fields "common" and "isParallel" and can panic if those
fields are missing or shapes change; update the function to defensively check
each reflection step (ensure ValueOf(t) is valid, use reflect.Indirect safely,
check FieldByName returns a valid, settable Value and call
IsValid/CanInterface/Kind before accessing), return false if any check fails,
and optionally wrap the reflection block with a deferred recover that returns
false on panic to guarantee no runtime panic from structural changes.
| type mockT struct { | ||
| cleanupWasCalled bool | ||
| } | ||
|
|
||
| func (m *mockT) Cleanup(f func()) { | ||
| m.cleanupWasCalled = true | ||
| f() | ||
| } | ||
|
|
||
| func TestPushWithCleanup(t *testing.T) { | ||
| og := stacks.frames | ||
| defer func() { | ||
| stacks.frames = og | ||
| }() | ||
|
|
||
| t.Run("cleanup called", func(t *testing.T) { | ||
| mock := &mockT{} | ||
|
|
||
| // ensure we start with no stacks | ||
| stacks.frames = map[uintptr][]testframe{} | ||
|
|
||
| PushWithCleanup(mock) | ||
|
|
||
| if !mock.cleanupWasCalled { | ||
| t.Errorf("expected cleanup to be called") | ||
| } | ||
| }) | ||
|
|
||
| t.Run("called with non-cleanup type", func(t *testing.T) { | ||
| defer func() { | ||
| r := recover() | ||
| if r == nil { | ||
| t.Error("expected panic, got nil") | ||
| } else if err, ok := r.(error); !ok || !errors.Is(err, ErrNoCleanupFunction) { | ||
| t.Errorf("expected panic with ErrNoCleanupFunction, got: %v", r) | ||
| } | ||
| }() | ||
|
|
||
| // ensure we start with no stacks | ||
| PushWithCleanup(42) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
PushWithCleanup tests correctly exercise cleanup invocation and non-cleanup panic.
The mockT type confirms that PushWithCleanup calls the Cleanup(func()) hook, and the second subtest verifies that passing a non-cleanup type panics with ErrNoCleanupFunction. Functionally this looks good. Minor nit: in the "called with non-cleanup type" subtest the comment says “ensure we start with no stacks” but stacks.frames isn’t reset there—if you want the comment to be strictly accurate, you could mirror the first subtest’s initialisation:
- // ensure we start with no stacks
- PushWithCleanup(42)
+ // ensure we start with no stacks
+ stacks.frames = map[uintptr][]testframe{}
+ PushWithCleanup(42)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type mockT struct { | |
| cleanupWasCalled bool | |
| } | |
| func (m *mockT) Cleanup(f func()) { | |
| m.cleanupWasCalled = true | |
| f() | |
| } | |
| func TestPushWithCleanup(t *testing.T) { | |
| og := stacks.frames | |
| defer func() { | |
| stacks.frames = og | |
| }() | |
| t.Run("cleanup called", func(t *testing.T) { | |
| mock := &mockT{} | |
| // ensure we start with no stacks | |
| stacks.frames = map[uintptr][]testframe{} | |
| PushWithCleanup(mock) | |
| if !mock.cleanupWasCalled { | |
| t.Errorf("expected cleanup to be called") | |
| } | |
| }) | |
| t.Run("called with non-cleanup type", func(t *testing.T) { | |
| defer func() { | |
| r := recover() | |
| if r == nil { | |
| t.Error("expected panic, got nil") | |
| } else if err, ok := r.(error); !ok || !errors.Is(err, ErrNoCleanupFunction) { | |
| t.Errorf("expected panic with ErrNoCleanupFunction, got: %v", r) | |
| } | |
| }() | |
| // ensure we start with no stacks | |
| PushWithCleanup(42) | |
| }) | |
| } | |
| type mockT struct { | |
| cleanupWasCalled bool | |
| } | |
| func (m *mockT) Cleanup(f func()) { | |
| m.cleanupWasCalled = true | |
| f() | |
| } | |
| func TestPushWithCleanup(t *testing.T) { | |
| og := stacks.frames | |
| defer func() { | |
| stacks.frames = og | |
| }() | |
| t.Run("cleanup called", func(t *testing.T) { | |
| mock := &mockT{} | |
| // ensure we start with no stacks | |
| stacks.frames = map[uintptr][]testframe{} | |
| PushWithCleanup(mock) | |
| if !mock.cleanupWasCalled { | |
| t.Errorf("expected cleanup to be called") | |
| } | |
| }) | |
| t.Run("called with non-cleanup type", func(t *testing.T) { | |
| defer func() { | |
| r := recover() | |
| if r == nil { | |
| t.Error("expected panic, got nil") | |
| } else if err, ok := r.(error); !ok || !errors.Is(err, ErrNoCleanupFunction) { | |
| t.Errorf("expected panic with ErrNoCleanupFunction, got: %v", r) | |
| } | |
| }() | |
| // ensure we start with no stacks | |
| stacks.frames = map[uintptr][]testframe{} | |
| PushWithCleanup(42) | |
| }) | |
| } |
🤖 Prompt for AI Agents
internal/testframe/stack_test.go around lines 192 to 233: the second subtest
comments claim "ensure we start with no stacks" but it does not reset
stacks.frames like the first subtest; to make the comment accurate and avoid
cross-test state leakage, set stacks.frames = map[uintptr][]testframe{} at the
start of the "called with non-cleanup type" subtest (mirror the first subtest),
keeping the rest of the panic assertion intact.
| // PushWithCleanup adds a new test frame to the current goroutine's stack | ||
| // and registers a cleanup function to remove it when the test frame completes. | ||
| // | ||
| // The function is safe to call concurrently and will not modify the stack | ||
| // of other goroutines. | ||
| func PushWithCleanup(t any) { | ||
| c, ok := t.(interface{ Cleanup(func()) }) | ||
| if !ok { | ||
| panic(ErrNoCleanupFunction) | ||
| } | ||
|
|
||
| Push(t) | ||
| c.Cleanup(Pop) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
PushWithCleanup correctly couples frames to test cleanup (minor style tweak possible)
The PushWithCleanup implementation cleanly enforces the presence of a Cleanup(func()) method, pushes the frame, and arranges for Pop to run via the test cleanup, which matches the intended lifecycle. You could optionally reuse the existing Cleanup interface type instead of the inline interface{ Cleanup(func()) } for consistency, but the current code is functionally sound.
🤖 Prompt for AI Agents
internal/testframe/stack.go around lines 129 to 142: currently the code uses an
inline type assertion c, ok := t.(interface{ Cleanup(func()) }) before calling
Push and c.Cleanup(Pop); change this to use the existing Cleanup interface type
(the package-level declared interface) instead of the inline anonymous interface
for consistency and clarity—i.e., perform the type assertion against Cleanup,
keep the panic on missing method, call Push(t) and then c.Cleanup(Pop) as
before.
| // type name, when possible | ||
| if n == "<nil>" { | ||
| fn := func(T) { /* NO-OP */ } | ||
| n = reflect.TypeOf(fn).In(0).Name() |
There was a problem hiding this comment.
Fix pipeline failure: use reflect.TypeFor instead of reflect.TypeOf
The golangci-lint modernize check is failing because reflect.TypeOf(fn).In(0) can be simplified to reflect.TypeFor[T]().
Apply this diff to fix the pipeline failure:
- n = reflect.TypeOf(fn).In(0).Name()
-
- // call the function to ensure it is considered "covered" by tests
- fn(t)
+ n = reflect.TypeFor[T]().Name()Note: This also eliminates the need for the dummy function call at line 40, which was only needed for test coverage of the old approach.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: release
[error] 37-37: golangci-lint: reflect.TypeOf call can be simplified using TypeFor (modernize) in internal/type-name.go:37
🪛 GitHub Check: module-qa / qa / run tests
[failure] 37-37:
reflecttypefor: reflect.TypeOf call can be simplified using TypeFor (modernize)
🤖 Prompt for AI Agents
In internal/type-name.go around line 37, replace the use of
reflect.TypeOf(fn).In(0).Name() with the modern reflect.TypeFor[T]() pattern to
satisfy golangci-lint modernize; update the code to call
reflect.TypeFor[ExpectedType]().Name() (or the appropriate generic type
parameter) and remove the now-unnecessary dummy function call at line 40 that
existed solely for the old reflection approach.
| func ExampleHaveContextValue() { | ||
| // this is needed to make the example work; this would be usually | ||
| // be `With(t)` where `t` is the *testing.T | ||
| test.Example() |
There was a problem hiding this comment.
Fix small typo in example comment.
Minor wording tweak to avoid the duplicated “be” while keeping the emphasis on typical With(t) usage.
- // this is needed to make the example work; this would be usually
- // be `With(t)` where `t` is the *testing.T
+ // this is needed to make the example work; this would usually
+ // be `With(t)` where `t` is the *testing.T📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func ExampleHaveContextValue() { | |
| // this is needed to make the example work; this would be usually | |
| // be `With(t)` where `t` is the *testing.T | |
| test.Example() | |
| func ExampleHaveContextValue() { | |
| // this is needed to make the example work; this would usually | |
| // be `With(t)` where `t` is the *testing.T | |
| test.Example() |
🤖 Prompt for AI Agents
In match-context_test.go around lines 87 to 90, the example comment contains a
duplicated word "be" ("would be usually be `With(t)`"); fix the typo by
rephrasing the comment to "would usually be `With(t)` where `t` is the
*testing.T" (or equivalent) so the sentence reads smoothly and retains the
emphasis on typical With(t) usage.
| func ExampleFalse() { | ||
| test.Example() | ||
|
|
||
| a := 1 | ||
|
|
||
| // only one test failure is reported by these tests | ||
|
|
||
| require.False(a == 1) // will fail and stop the test here | ||
| expect.False(a+1-1 == 1) // this would also fail but is not evaluated | ||
|
|
||
| // Output: | ||
| // expected false | ||
| } | ||
|
|
||
| func ExampleTrue() { | ||
| test.Example() | ||
|
|
||
| a := 1 | ||
|
|
||
| // only one test failure is reported by these tests | ||
|
|
||
| require.True(a == 0) // will fail and stop the test here | ||
| expect.True(a < 0) // this would also fail but is not evaluated | ||
|
|
||
| // Output: | ||
| // expected true | ||
| } |
There was a problem hiding this comment.
ExampleFalse/ExampleTrue Output lines likely missing the “got …” suffix
Given that the underlying BeFalse/BeTrue matchers currently report failures as "expected false, got true" and "expected true, got false" in the expect examples, the Output sections here showing only "expected false" / "expected true" are probably incorrect and will cause the examples to fail when run.
Consider updating the Output blocks to match the matcher output:
@@ func ExampleFalse() {
- // Output:
- // expected false
+ // Output:
+ // expected false, got true
@@ func ExampleTrue() {
- // Output:
- // expected true
+ // Output:
+ // expected true, got false
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func ExampleFalse() { | |
| test.Example() | |
| a := 1 | |
| // only one test failure is reported by these tests | |
| require.False(a == 1) // will fail and stop the test here | |
| expect.False(a+1-1 == 1) // this would also fail but is not evaluated | |
| // Output: | |
| // expected false | |
| } | |
| func ExampleTrue() { | |
| test.Example() | |
| a := 1 | |
| // only one test failure is reported by these tests | |
| require.True(a == 0) // will fail and stop the test here | |
| expect.True(a < 0) // this would also fail but is not evaluated | |
| // Output: | |
| // expected true | |
| } | |
| func ExampleFalse() { | |
| test.Example() | |
| a := 1 | |
| // only one test failure is reported by these tests | |
| require.False(a == 1) // will fail and stop the test here | |
| expect.False(a+1-1 == 1) // this would also fail but is not evaluated | |
| // Output: | |
| // expected false, got true | |
| } | |
| func ExampleTrue() { | |
| test.Example() | |
| a := 1 | |
| // only one test failure is reported by these tests | |
| require.True(a == 0) // will fail and stop the test here | |
| expect.True(a < 0) // this would also fail but is not evaluated | |
| // Output: | |
| // expected true, got false | |
| } |
🤖 Prompt for AI Agents
In require/bool_test.go around lines 9 to 35, the ExampleFalse and ExampleTrue
Output blocks only contain "expected false"/"expected true" but the
BeFalse/BeTrue matchers emit messages like "expected false, got true" and
"expected true, got false"; update the Output comments to include the full
matcher output (e.g., add ", got true" or ", got false" as appropriate) so the
example tests match the actual failure strings.
Summary by CodeRabbit
Breaking Changes
New Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.