Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

proposal: testing/slogtest: Add RunWithSkip func #65400

Closed
arukiidou opened this issue Jan 31, 2024 · 4 comments
Closed

proposal: testing/slogtest: Add RunWithSkip func #65400

arukiidou opened this issue Jan 31, 2024 · 4 comments
Labels
FrozenDueToAge Proposal
Milestone
Proposal

Comments

@arukiidou
Copy link
Contributor

arukiidou commented Jan 31, 2024 •
edited
Loading

Proposal Summary

Allows skipping of tests with specific names

func RunWithSkip(t *testing.T, newHandler func(*testing.T) slog.Handler, result func(*testing.T) map[string]any, skipTestName ...string)

Problems

Several logging libraries trying to support log/slog, but seem to not much imported testing/slogtest

Background

This is just a guess based on my personal insight:

Proposal

Add RunWithSkip, allows skipping of tests with specific test names.

Benefits

  • Expecting more slogtest imports.
  • It makes possible "partial slog support" approach
    • I think that partial approach is better than all-or-nothing.
    • For example, kubernetes gateway-api conformance supports skipping tests.
@gopherbot gopherbot added this to the Proposal milestone Jan 31, 2024
@arukiidou arukiidou changed the title proposal: testing/slogtest: Add RunWithSkip proposal: testing/slogtest: Add RunWithSkip func Jan 31, 2024
@ianlancetaylor
Copy link
Member

CC @jba

@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Feb 7, 2024
@arukiidou
Copy link
Contributor Author

arukiidou commented Feb 8, 2024 •
edited
Loading

If could not skip the test,
Future additions of slogtest cases that support edge cases, it might break the GHA pipeline and block Golang updates.

@jba
Copy link
Contributor

jba commented Feb 20, 2024

I think you can do what you want with the existing slogtest.Run function:

slogtest.Run(t, func(t *testing.T) slog.Handler {
    if skipNameMap[t.Name()] {
        t.Skip()
   }
   ...,
}, ...)

Please correct me if I've misunderstood.

@arukiidou
Copy link
Contributor Author

you are right,
This proposal is retractable.

https://go.dev/play/p/nJY0XtnW6mz

newHandler := func(tt *testing.T) slog.Handler {
if slices.Contains([]string{"TestSlogtest/empty-attr", "TestSlogtest/inline-group", "TestSlogtest/empty-group-record"}, tt.Name()) {
tt.Skip("skipped.")
}
buff.Reset()
return zapslog.NewHandler(core, nil)
}
run := func(tt *testing.T) map[string]any {
var ent map[string]any
if err := json.Unmarshal(buff.Bytes(), &ent); err != nil {
t.Fatal(err)
}
return ent
}
slogtest.Run(t, newHandler, tf)

@pohly pohly mentioned this issue Jan 21, 2025
Open
@golang golang locked and limited conversation to collaborators Feb 20, 2025
@aclements aclements removed this from Proposals Feb 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal
Projects
None yet
Milestone
Proposal
Development

No branches or pull requests
4 participants
@ianlancetaylor @gopherbot @arukiidou @jba