-
Notifications
You must be signed in to change notification settings - Fork 231
Unit tests restructuring #370
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
Conversation
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.
Great work, this modernizes the test infrastructure. I noticed some TODO's were remaining, but my guess is that it's why this PR is still in draft.
Thanks! This PR is not marked as draft. It is #371 that is marked as draft. |
For reference here are the TODO I was talking about https://github.com/microsoftgraph/msgraph-sdk-javascript/pull/370/files#diff-d4ad0dac051d781735e98ebecb49ec8c172723a92f37910c1c413c414dd2df23 |
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.
with comments
CONTRIBUTING.md
Outdated
|
||
Once you have done with your changes, You have to build and test your changes | ||
To build the library run, | ||
Once you have done with your changes, You have to build and test your changes To build the library run, |
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 for grammar, punctuation, and voice (passive)
Once you have done with your changes, You have to build and test your changes To build the library run,
becomes
Build and test your changes with the following commands after you have completed your work:
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.
Done
test/node/NodeTests.md
Outdated
|
||
Karma is used for testing the code in browsers. To run the unit tests using karma run the following steps - | ||
|
||
- Run the script npm install |
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
Run the script npm install
Run the script npm run test
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.
Done
This PR focusses to improve the existing test structure of the project. Changes are as follows -
a.
Karma
- To test the unit test in browsersb.
@istanbuljs/nyc-config-typescript
- Evaluation of the test coveragea. Renamed from folder spec/ to test/
b. Divided unit test in folders - common, node and browser.
c. Removed the config files from the test folder.
esm
dependency to test run mocha tests on transpiled ESM modules.fetch api
. Updated the isomorphic-fetch dependency.TODO - Check if the any changes are needed in the GraphResponseHandler to adhere to fetch api/http/ Graph Response standards. Will create a follow up PR if any changes are present