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.

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

micrometer-observation-test support for assertions on events #5576

Closed
wabrit opened this issue Oct 9, 2024 · 7 comments · Fixed by #5605
Closed

micrometer-observation-test support for assertions on events #5576

wabrit opened this issue Oct 9, 2024 · 7 comments · Fixed by #5605

Comments

@wabrit
Copy link
Contributor

wabrit commented Oct 9, 2024

Please describe the feature request.
The micrometer-observation-test dependency does not appear to offer a simple way of asserting on observation events in tests

Rationale
The TestObservationRegistryAssert class handles assertions on what observations have been made, the key values associated with an observation, etc. But it does not appear to provide support for asserting that particular calls to Observation#event(...) have been made.

This is necessary to provide test coverage for observations which raise events.

Additional context
This is based on using version 1.13.2 of io.micrometer:micrometer-observation-test

@marcingrzejszczak marcingrzejszczak added enhancement A general enhancement and removed waiting-for-triage labels Oct 9, 2024
@shakuzen shakuzen added help wanted An issue that a contributor can help us with module: micrometer-observation-test labels Oct 11, 2024
@shakuzen shakuzen added this to the 1.x milestone Oct 11, 2024
@shakuzen
Copy link
Member

@wabrit thanks for opening the issue. It sounds like a useful enhancement. Would you be interested in contributing a pull request for this?

@wabrit
Copy link
Contributor Author

wabrit commented Oct 11, 2024

Hi @shakuzen - I'd be happy to try, and I do have my own "workaround", but perhaps it would be a good idea to submit some kind of API design proposal first before a PR? Is here the right place to do that?

@shakuzen
Copy link
Member

Sure, here is fine. Leave a comment with what you have in mind.

@wabrit
Copy link
Contributor Author

wabrit commented Oct 11, 2024

What I envisaged would be to extend the TestObservationRegistryAssert class (and supporting classes) to support this:

TestObservationRegistryAssert.assertThat(observationRegistry)
        .hasObservationWithNameEqualTo("foo")
        .that()
        .hasEvent("event1") /* event with name event1 exists in observation */
        .doesNotHaveEvent("event2") /* no event with name event2 in observation */
        ....

@wabrit
Copy link
Contributor Author

wabrit commented Oct 21, 2024

Hi all - does the above proposal sound ok before I embark on a PR, and (not having committed to this project before) is there a guide to that process? (I assume I need to fork the repo and create a branch on the fork to submit the PR against)

@shakuzen
Copy link
Member

Your proposal looks reasonable to me. As for contributing, check the contributing guide. Yes, forking the repo and committing the changes to a branch in that fork and submitting a pull request would be the right process.

@wabrit
Copy link
Contributor Author

wabrit commented Oct 24, 2024

PR raised: #5605

@shakuzen shakuzen removed the help wanted An issue that a contributor can help us with label Oct 25, 2024
@shakuzen shakuzen modified the milestones: 1.x, 1.15.0-M1 Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants