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.

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

Add test for setting the external_id parameter #1038

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

matthewcarroll
Copy link

Changes

This PR adds a test for setting the external_id parameter when bulk importing users.

The lcov report shows that the test added on this branch increased the Branches coverage from 93.75% to 100% for the file src/management/__generated/managers/jobs-manager.ts.

Screenshot of coverage on this branch:
pr-branch-coverage

Screenshot of coverage on master:
master-coverage

I'm happy to change or consolidate anything else in this file if you'd like.

Thank you!

References

Bulk importing users

Testing

Run npm test to run the new test in this PR.

Run npm run test:ci to generate the code coverage report for this branch, shown in the screenshot above.

  • This change adds unit test coverage
  • This change adds integration test coverage

Checklist

@matthewcarroll matthewcarroll requested a review from a team as a code owner September 13, 2024 16:31
})
.reply(200, {});

jobs.importUsers(Object.assign({}, data, { external_id })).then(() => {
Copy link
Author

Choose a reason for hiding this comment

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

Using Object.assign instead of spread here for style consistency with other tests in this file.

Copy link
Contributor

@tusharpandey13 tusharpandey13 left a comment

Choose a reason for hiding this comment

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

LGTM

@tusharpandey13
Copy link
Contributor

Thanks @matthewcarroll, can you sign your commits and update this branch, we'll get this merged.

@matthewcarroll matthewcarroll force-pushed the jobs-manager-test branch 2 times, most recently from 6f39c1e to 7db15a4 Compare November 18, 2024 21:57
@matthewcarroll
Copy link
Author

Thank you @tusharpandey13!

I signed the commit and rebased it onto node-auth0/master.
I can redo it with a merge-commit if you'd prefer.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants