-
Notifications
You must be signed in to change notification settings - Fork 276
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
Filter continueConversation events when logging #2903
Filter continueConversation events when logging #2903
Conversation
}).use(new TranscriptLoggerMiddleware(new SyncErrorLogger())); | ||
|
||
adapter.send('test'); | ||
describe("'s error handling", function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference, this is the major test refactor portion.
d448d37
to
d7d764c
Compare
d7d764c
to
2311b83
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the actual values of constants, enums, etc we do stay aligned with what's used in C#/other languages. Once the .NET PR lands the The ActivityEventNames enum in JS should match what's in .NET. Other than that, the PR looks solid!
As a general rule, if it's a value that might be serialized and could be shared across languages, it should be aligned across all languages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Fixes #2805
This PR also includes a small test refactoring to use
sinon
for mocking and verifying error handling. It's a more natural way to set up expectations and verify them without patching methods. It was a bit of bikeshedding, but it should serve as a useful example for future testing scenarios.