Skip to content

TextFormatter: cover nil pointer method receivers - #1584

Merged
thaJeztah merged 1 commit into
sirupsen:masterfrom
thaJeztah:more_coverage
Aug 19, 2026
Merged

thaJeztah merged 1 commit into
sirupsen:masterfrom
thaJeztah:more_coverage

Conversation

@thaJeztah

@thaJeztah thaJeztah commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Extend the regression test for panicking Error and String methods to cover nil pointers with both pointer- and value-receiver implementations.

This includes the value-receiver case that triggered the reported regression, such as *net.IP.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Extends Logrus’ TextFormatter regression test coverage around error / fmt.Stringer formatting to include nil pointer values where the interface method set comes from either pointer-receiver or value-receiver implementations, ensuring formatting remains non-panicking and stable.

Changes:

  • Expands TestTextFormatterPanickingValue table cases to include nil pointers that satisfy error / fmt.Stringer via value-receiver methods (in addition to pointer-receiver methods).
  • Adds dedicated helper types (nilValueError, nilValueStringer) to model the value-receiver-on-nil-pointer scenario described in the regression context.
  • Clarifies test case descriptions to differentiate pointer-receiver vs value-receiver nil scenarios.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Extend the regression test for panicking Error and String methods to cover
nil pointers with both pointer- and value-receiver implementations.

This includes the value-receiver case that triggered the reported
regression, such as *net.IP.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
thaJeztah merged commit 0b920ad into sirupsen:master Aug 19, 2026
13 checks passed
@thaJeztah
thaJeztah deleted the more_coverage branch August 19, 2026 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants