Skip to content

Conversation

@marun
Copy link
Contributor

@marun marun commented Nov 20, 2024

Why this should be merged

Previously testing used an incoherent mix of TestContext.Outf, fmt.Print and fmt.Fprint. This PR switches everything to use the zap logger for consistency with the rest of avalanchego.

How this works

  • switches fixture like tmpnet and the e2e helpers to zap
  • updates all tests to use zap

How this was tested

CI

TODO

  • Figure out why the logger returned by NewSimpleLogger is outputting weird level e.g. Level(-6) instead of INFO

Need to be documented in RELEASES.md?

N/A

@marun marun added the testing This primarily focuses on testing label Nov 20, 2024
@marun marun self-assigned this Nov 20, 2024
@marun marun marked this pull request as ready for review November 20, 2024 20:38
tests/log.go Outdated
Comment on lines 23 to 24
// TODO(marun) Figure out why this is outputting e.g. `Level(-6)` instead of `INFO`
EncodeLevel: zapcore.LowercaseLevelEncoder,
Copy link
Contributor

Choose a reason for hiding this comment

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

We have modified how levels are encoded. We should be providing a custom leveler like in the rest of avalanchego:

func levelEncoder(l zapcore.Level, enc zapcore.PrimitiveArrayEncoder) {
	enc.AppendString(Level(l).String())
}

func jsonLevelEncoder(l zapcore.Level, enc zapcore.PrimitiveArrayEncoder) {
	enc.AppendString(Level(l).LowerString())
}

See utils/logging/format.go#newTermEncoderConfig

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any objection to exporting these functions so they can be reused?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope - makes sense to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

maru-ava and others added 3 commits November 21, 2024 20:24
@marun marun force-pushed the testing-switch-to-zap branch from cf8a4d4 to 292016a Compare November 21, 2024 19:27
@marun marun added this pull request to the merge queue Nov 21, 2024
Merged via the queue into master with commit 5cf58be Nov 21, 2024
23 checks passed
@marun marun deleted the testing-switch-to-zap branch November 21, 2024 22:39
tsachiherman pushed a commit that referenced this pull request Jan 29, 2025
Signed-off-by: marun <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing This primarily focuses on testing

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants