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

test: add basic unit test - OKTA-327875 #149

Closed
wants to merge 5 commits into from

Conversation

shuowu
Copy link
Contributor

@shuowu shuowu commented Nov 16, 2020

Add basic unit tests for demo purposes per how to handle okta-auth-js dep issue.

Resolves: #129

"okta-hosted-login-server": "npm start --prefix okta-hosted-login/",
"test:okta-hosted-login": "export TEST_TYPE=implicit || setx TEST_TYPE \"implicit\" && protractor okta-oidc-tck/e2e-tests/okta-hosted-login/conf.js",
"custom-login-server": "npm start --prefix custom-login/",
"test:custom-login": "export TEST_TYPE=implicit || setx TEST_TYPE \"implicit\" && protractor okta-oidc-tck/e2e-tests/custom-login/conf.js",
"resource-server": "node scripts/node-resource-server.js",
"test:unit": "npm run test --prefix okta-hosted-login/ && npm run test --prefix custom-login/",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not 100% sold on having the root level use npm run test:unit while the individual packages use npm run test, but I'm not 100% against it either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how about use test:unit in both places for consistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After checking the code generated from create-react-app, I think it might make sense to keep the current naming conventions. As test in each individual package matches the script generated from CRA. And the root level test:unit is for the CI usage.

let container;

beforeEach(() => {
container = document.createElement('div');
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's copied from https://github.com/mraible/schematics/blob/main/src/add-auth/react/src/App.test.js
looks like mount should be enough here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

after checking the doc, it's the suggested way from react test utils
Ref: https://reactjs.org/docs/test-utils.html#act

@shuowu shuowu requested a review from swiftone November 16, 2020 22:11
@shuowu-okta shuowu-okta force-pushed the sw-add-basic-unit-test-OKTA-327875 branch from b3526d1 to 47d4c95 Compare November 23, 2020 17:57
@shuowu
Copy link
Contributor Author

shuowu commented Nov 23, 2020

E2E passed in bacon:

Screen Shot 2020-11-23 at 1 37 07 PM

@shuowu-okta shuowu-okta force-pushed the sw-add-basic-unit-test-OKTA-327875 branch from 47d4c95 to 21ed679 Compare November 23, 2020 18:47
},
"jest": {
"moduleNameMapper": {
"^@okta/okta-auth-js$": "<rootDir>/node_modules/@okta/okta-auth-js/dist/okta-auth-js.min.js"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"^@okta/okta-auth-js$": "<rootDir>/node_modules/@okta/okta-auth-js/dist/okta-auth-js.min.js"
"^@okta/okta-auth-js$": "<rootDir>/node_modules/@okta/okta-auth-js/dist/okta-auth-js.umd.js"

This is what fixed the tests for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finally figured out I was missing the MemoryRouter from react-router-dom for jest test.

},
"jest": {
"moduleNameMapper": {
"^@okta/okta-auth-js$": "<rootDir>/node_modules/@okta/okta-auth-js/dist/okta-auth-js.min.js"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"^@okta/okta-auth-js$": "<rootDir>/node_modules/@okta/okta-auth-js/dist/okta-auth-js.min.js"
"^@okta/okta-auth-js$": "<rootDir>/node_modules/@okta/okta-auth-js/dist/okta-auth-js.umd.js"

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.

Add a sample test to prove tests work when Okta is integrated
4 participants