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.

Sign up for GitHub

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

Jump to bottom

Fix Telemetry Cache Hit Bug #2170

Merged
merged 4 commits into from Aug 20, 2020
Merged

Fix Telemetry Cache Hit Bug #2170

merged 4 commits into from Aug 20, 2020

Conversation

tnorling
Copy link
Collaborator

If ServerTelemetryManager is not instantiated, getting a token from cache in SilentFlowClient would throw an error due to the incrementCacheHits call not being gated. This PR fixes that bug and adds a test for getting token from cache

@github-actions github-actions bot added the msal-common Related to msal-common package label Aug 20, 2020
sameerag
sameerag approved these changes Aug 20, 2020
View reviewed changes
Copy link
Member

@sameerag sameerag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. Thanks for the tests.

General suggestion, the cache tests in node contain a mock objects class. We can do something similar for common tests.

@coveralls
Copy link

coveralls commented Aug 20, 2020 •
edited

Coverage Status

Coverage increased (+0.08%) to 82.242% when pulling 89ec062 on Fix-Telem-CacheHit-bug into 98c3f78 on dev.

@tnorling tnorling merged commit 48cd24b into dev Aug 20, 2020
@tnorling tnorling deleted the Fix-Telem-CacheHit-bug branch October 19, 2020 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonnutter jasonnutter jasonnutter approved these changes

@sameerag sameerag sameerag approved these changes

@DarylThayil DarylThayil Awaiting requested review from DarylThayil

@jmckennon jmckennon Awaiting requested review from jmckennon

@jo-arroyo jo-arroyo Awaiting requested review from jo-arroyo jo-arroyo is a code owner

@pkanher617 pkanher617 Awaiting requested review from pkanher617

@hectormmg hectormmg Awaiting requested review from hectormmg hectormmg is a code owner

Assignees
No one assigned
Labels
msal-common Related to msal-common package
Projects
None yet
Milestone
@azure/msal-node@1.0.0-alpha.5
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tnorling @coveralls @jasonnutter @sameerag