Improve abort tests across all SDKs; add Go unsubscribe tests #48
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
SteveSandersonMS
force-pushed
the
stevesa/session-unsubscribe-tests
branch
from
e0e1822 to
1ffd814
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive unit tests for the Session.On() event handler subscription and unsubscription functionality. The tests are designed to validate the fix from PR #24, which addressed an issue with invalid function pointer comparison in the unsubscribe mechanism.
Changes:
- Added new test file
go/session_test.gowith 5 test cases covering event handler subscription/unsubscription - Tests verify multiple handlers, selective unsubscription, idempotent unsubscribe, handler ordering, and concurrent safety
Comments suppressed due to low confidence (4)
go/session_test.go:81
- The type
sessionHandlerdoes not exist. The Session struct's handlers field is of type[]SessionEventHandler(uppercase 'S'). This will cause a compilation error. ChangesessionHandlertoSessionEventHandler.
handlers: make([]sessionHandler, 0),
go/session_test.go:98
- The type
sessionHandlerdoes not exist. The Session struct's handlers field is of type[]SessionEventHandler(uppercase 'S'). This will cause a compilation error. ChangesessionHandlertoSessionEventHandler.
handlers: make([]sessionHandler, 0),
go/session_test.go:11
- The type
sessionHandlerdoes not exist. The Session struct's handlers field is of type[]SessionEventHandler(uppercase 'S'). This will cause a compilation error. ChangesessionHandlertoSessionEventHandler.
handlers: make([]sessionHandler, 0),
go/session_test.go:29
- The type
sessionHandlerdoes not exist. The Session struct's handlers field is of type[]SessionEventHandler(uppercase 'S'). This will cause a compilation error. ChangesessionHandlertoSessionEventHandler.
handlers: make([]sessionHandler, 0),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
SteveSandersonMS
changed the title
SteveSandersonMS
force-pushed
the
stevesa/session-unsubscribe-tests
branch
from
1ffd814 to
520d8fb
Compare
SteveSandersonMS
changed the title
The unsubscribe function was failing due to invalid function pointer comparisons. Refactored handler registration to use unique IDs for reliable cleanup. Tests verify: - Multiple handlers can be registered and all receive events - Unsubscribing one handler doesn't affect others - Calling unsubscribe multiple times is safe - Handlers are called in registration order - Concurrent subscribe/unsubscribe is safe Co-authored-by: nathfavour <116535483+nathfavour@users.noreply.github.com>
SteveSandersonMS
force-pushed
the
stevesa/session-unsubscribe-tests
branch
from
520d8fb to
3a8c2a5
Compare
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
SteveSandersonMS
changed the title
SteveSandersonMS
changed the title
SteveSandersonMS
changed the title
friggeri
approved these changes
Jan 20, 2026
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.
Continuation of #24 (thanks, @nathfavour!)
Previously we didn't have proper
aborttests because the CLI didn't produce the events we expected. But now it does.