Allow creation of OutputCallbackInfo and InputCallbackInfo objects (for test purposes) #899
Conversation
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
This allows StreamInstants to be created, which enables testing the data callback function with unit tests.
Looks like making the fields public means they are now exposed in JS so they need wasm_bindgen I added the attribute on the input side too: the CI test only needs the output side but it only makes sense to have it both ways.
|
Fixed the emscripten issue, please take a look and let me know if I need to do anything else. |
This was referenced Jan 26, 2025
Closed
|
It's been a while - sorry for that - but new maintainer here and I follow your rationale. Would you resolve the conflicts and update the PR? |
|
For consideration: instead of making everything |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 change makes the fields of
StreamInstant,OutputCallbackInfoandInputCallbackInfopub.The intent is to allow
OutputCallbackInfoinstances to be created, which then allows unit testing the data callback function. Because it is a self-contained function that simply fills data in a buffer, it is, in principle, very unit-testable -- but it's very difficult to do so today because the test code cannot create anOutputCallbackInfofor it.I don't believe there's any good reason for these fields to not be pub. In particular, I noticed the fields of
OutputStreamTimestampare public, so the "middle" layer is already creatable, just notStreamInstantandOutputCallbackInfo. Also, the structures are pretty plain data objects with no special logic or anything like that, so there really is no harm in allowing them to be created.