Skip to content

Add tests for SwiftLogNoOpLogHandler#449

Merged
kukushechkin merged 2 commits intoapple:mainfrom
crleonard:add-no-op-log-handler-tests
Apr 27, 2026
Merged

Add tests for SwiftLogNoOpLogHandler#449
kukushechkin merged 2 commits intoapple:mainfrom
crleonard:add-no-op-log-handler-tests

Conversation

@crleonard
Copy link
Copy Markdown
Contributor

Add tests for SwiftLogNoOpLogHandler

Motivation:

SwiftLogNoOpLogHandler is a public type with no dedicated test coverage.

Modifications:

  • Add tests for both initialisers, logLevel, metadata, the metadata subscript, log(event:), and end-to-end usage via Logger.

Result:

SwiftLogNoOpLogHandler has test coverage on par with other handlers.
Coverage from 89% over all tests, to 90%.

Add tests for SwiftLogNoOpLogHandler

### Motivation:

`SwiftLogNoOpLogHandler` is a public type with no dedicated test coverage.

### Modifications:

* Add tests for both initialisers, `logLevel`, `metadata`, the metadata subscript, `log(event:)`, and end-to-end usage via `Logger`.

### Result:

`SwiftLogNoOpLogHandler` has test coverage on par with other handlers.
@kukushechkin kukushechkin added the semver/none No version bump required. label Apr 27, 2026
@kukushechkin
Copy link
Copy Markdown
Contributor

@crleonard thank you for the contribution! I understand the motivation to fill the coverage gap, but I think the main value in testing NoOpLogHandler would be to verify it didn't do anything in logEventIsDiscarded and loggerUsingNoOpHandlerSilentlyDiscards. Right now these tests have no assertions and test this log handler does not crash. I don't see any reasonable way of doing, so recommend to change the misleading test names — the implementation does not verify anything is discarded.

@crleonard
Copy link
Copy Markdown
Contributor Author

crleonard commented Apr 27, 2026

@kukushechkin thanks for the quick review, updated the test names.

@kukushechkin kukushechkin merged commit deae26e into apple:main Apr 27, 2026
115 of 117 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver/none No version bump required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants