-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Added msal-node client-credential regression test #6276
Conversation
"author": "", | ||
"license": "MIT", | ||
"dependencies": { | ||
"@azure/msal-node": "^1.18.0", |
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.
v1?
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.
We should probably include this in workspaces so you pull the local versions of msal-node (and also remove package-lock)
clientId: "client_id", | ||
authority: "https://login.microsoftonline.com/tenant_id", | ||
knownAuthorities: ["https://login.microsoftonline.com/tenant_id"], | ||
clientSecret: "client_secret", |
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.
nit: should probably also do one for certificates. maybe in another PR.
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.
Overall looks good. Just adding a couple minor comments below.
name: msal-node client-credential Regression Test | ||
tool: 'benchmarkjs' | ||
output-file-path: regression-tests/msal-node/client-credential/output.txt | ||
external-data-json-path: ./cache/client-credential-benchmark-data.json |
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.
FWIW, although this external-data-json-path
and the actions/cache@v3
above were mentioned in Continuous Benchmark's README, its actual github workflow yml file did not need to use such an external data file. I ended up not using this setting.
This PR introduces the beginnings of regression testing for msal-node confidential client flows; In this specific case, the client credential flow. BenchmarkJS is used to measure performance of msal-node.
Sample output of BenchmarkJS looks like:
If a regression of >10% is detected, github actions will leave a commit comment:
Example of commit comment when a fake regression was introduced to this PR.
Every time a PR is merged into dev, the latest output from "regression-tests/msal-node/client-credential/output.txt" (if available, if the client-credential test was run on the PR) will be saved to the branch "gh-pages", and will be available to visually see at: https://azuread.github.io/microsoft-authentication-library-for-js/dev/bench/.