Extend existing KV test with instrumentation assertions#4356
Merged
Conversation
fa71e49 to
e9105ae
Compare
fhanau
reviewed
Jun 17, 2025
fhanau
reviewed
Jun 17, 2025
fhanau
reviewed
Jun 17, 2025
fhanau
approved these changes
Jun 17, 2025
ad3dd54 to
935bdfa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This sets up a streaming tail worker for the existing KV tests, and then asserts against the telemetry that they emit today. Copies the approach in the tail worker tests.
There is no KV implementation in workerd so the existing KV tests mock each response. I could duplicate this logic into a separate test file to not disturb the existing tests, but thought that keeping everything together makes sense. As we extend the instrumentation logic this will likely encourage us to improve the coverage in the existing tests.