OKTA-656562 karma to jest #3418

glenfannin-okta · 2023-10-16T15:20:04Z

Description:

The purpose of this PR is to migrate all tests that currently use karma (referenced in legacy-tests.js) to use jest and to remove all karma dependencies.

PR Checklist

Have you verified the basic functionality for this change?
Did you add tests, as appropriate, following our Automated Test guidelines?
Did you follow our Security Best Practices?
Did you verify the change by running downstream monolith artifact? (Provide link to build below)
Does this PR include noticeable changes to the UI? (If yes, attach screenshots/video below)

Issue:

OKTA-656562

Reviewers:

Screenshot/Video:

Downstream Monolith Build:

.testcaferc.js

shawyu-okta · 2023-10-23T20:40:23Z

scripts/unit-jest.sh

@@ -7,7 +7,7 @@ export TEST_RESULT_FILE_DIR="${REPO}/build2/reports/unit"
 echo $TEST_SUITE_TYPE > $TEST_SUITE_TYPE_FILE
 echo $TEST_RESULT_FILE_DIR > $TEST_RESULT_FILE_DIR_FILE

-if ! yarn test --type jest --runInBand; then


was --runInBand causing issues? This is actually good though since it should run tests concurrently (faster overall)

Yea, using the --runInBand flag caused the test to timeout (it took > 30mins to actually complete). When I looked up what the flag was meant for I realized we did not need it in bacon (only during debuggin). This decreased the time to ~10 mins

test/unit/spec/v1/Animations_spec.js

shawyu-okta · 2023-10-25T23:37:32Z

test/unit/jest/jest-setup-global.js


+global.TextEncoder = TextEncoder;
+global.TextDecoder = TextDecoder;
+global.crypto = new Crypto();
 global.$ = global.jQuery = $;
 global.DEBUG = false;
 global.getJasmineRequireObj = function jestSetupGlobalJasmine() {
  return jasmine;
 };
 global.jasmine = jasmine;


We should follow up separately to remove jasmine as it's very confusing to know if a call to something like spyOn is from jasmine or from jest. Additionally, the interface of the returned Spy object is different between jasmine and jest's versions so would be good to migrate so we don't need to keep jasmine in our devDeps.

I will create a ticket for this, maybe we can actually add it as another UI Guild rotation task 🤔

test/unit/spec/v1/EnrollCall_spec.js

test/unit/spec/v1/EnrollSms_spec.js

shawyu-okta · 2023-10-26T17:31:34Z

test/unit/spec/v1/IDPDiscovery_spec.js

-          // Verify tooltip is gone
-          expect(test.form.isSecurityImageTooltipDestroyed()).toBe(true);
-        });
-    });


Are the deleted tests in this file moved someplace else or are they removed entirely?

Yes sir, moved into the PrimaryAuthForm_spec.js testcafe spec: https://github.com/okta/okta-signin-widget/pull/3418/files#diff-7fc65ff7a9deb6e52de14ecbe846e256708dd5e763b6a70031bd7cbe0f57a5b7R120-R226

test/unit/spec/v1/PrimaryAuth_spec.js

shawyu-okta · 2023-10-26T17:40:33Z

test/unit/spec/v1/PrimaryAuth_spec.js

-        .then(function() {
-          expect($.qtip.prototype.toggle.calls.argsFor(0)).toEqual(jasmine.objectContaining({ 0: true }));
-        });
-    });


deleted tests?

Moved into PrimaryAuthForm_spec.js testcafe spec: https://github.com/okta/okta-signin-widget/pull/3418/files#diff-7fc65ff7a9deb6e52de14ecbe846e256708dd5e763b6a70031bd7cbe0f57a5b7R120-R226

shawyu-okta · 2023-10-26T17:41:19Z

test/unit/spec/v1/PrimaryAuth_spec.js

@@ -3296,7 +3198,7 @@ Expect.describe('PrimaryAuth', function() {
    // cannot mock it because we defer to AuthJs to do set window.location.
    // On the plus side, there is an e2e test that covers this.
    // eslint-disable-next-line jasmine/no-disabled-tests
-    xit('redirects to the correct url in the social idp redirect flow');


should this stay uncommented?

Deleting this since there is already an e2e test that covers this.

shawyu-okta · 2023-10-26T17:43:29Z

test/unit/jest/jest-setup-global.js

+// patch jquery 3 visible pseudos selector issue in jsdom
+// https://github.com/jsdom/jsdom/issues/1048#issuecomment-401599392
+// https://github.com/jsdom/jsdom/issues/1048#issuecomment-595961496
+window.Element.prototype.getClientRects = function() {


Is there any workaround to avoiding using jquery to do the check for :visible instead of having to polyfill this behavior for jsdom?

We would need to upate code within Gen 1 to not use the visible selector, this is one example where we are using the visible selector, but it is also used by JQuery qtip.

test/unit/helpers/dom/AdminConsentRequiredForm.js

glenfannin-okta · 2023-10-26T20:47:37Z

test/unit/spec/v1/IDPDiscovery_spec.js

-    itp('sets aria-expanded attribute correctly when clicking help', function() {
-      return setup().then(function(test) {
-        expect(test.form.helpFooter().attr('aria-expanded')).toBe('false');
-        test.form.helpFooter().click();
-        expect(test.form.helpFooter().attr('aria-expanded')).toBe('true');
-      });
-    });


Moved into PrimaryAuthForm_spec.js

glenfannin-okta · 2023-10-26T20:48:40Z

test/unit/spec/v1/IDPDiscovery_spec.js

-    itp('does not show anti-phishing message if security image is hidden', function() {
-      return setup({ features: { securityImage: true } })
-        .then(function(test) {
-          test.setNextResponse(resSecurityImageFail);
-          test.form.securityBeaconContainer().hide();
-          spyOn($.qtip.prototype, 'toggle').and.callThrough();
-          test.form.setUsername('testuser@clouditude.net');
-          $(window).trigger('resize');
-          return waitForBeaconChange(test);
-        })
-        .then(function(test) {
-          expect($.qtip.prototype.toggle.calls.count()).toBe(1);
-          expect($.qtip.prototype.toggle.calls.argsFor(0)).toEqual(jasmine.objectContaining({ 0: false }));
-          $.qtip.prototype.toggle.calls.reset();
-          test.form.securityBeaconContainer().show();
-          $(window).trigger('resize');
-          return Expect.waitForSpyCall($.qtip.prototype.toggle);
-        })
-        .then(function() {
-          expect($.qtip.prototype.toggle.calls.count()).toBe(1);
-          expect($.qtip.prototype.toggle.calls.argsFor(0)).toEqual(jasmine.objectContaining({ 0: true }));
-        });
-    });
-    itp('show anti-phishing message if security image become visible', function() {
-      return setup({ features: { securityImage: true } })
-        .then(function(test) {
-          spyOn($.qtip.prototype, 'toggle').and.callThrough();
-          test.setNextResponse(resSecurityImageFail);
-          test.form.setUsername('testuser@clouditude.net');
-          return waitForBeaconChange(test);
-        })
-        .then(function(test) {
-          expect($.qtip.prototype.toggle.calls.argsFor(0)).toEqual(jasmine.objectContaining({ 0: true }));
-          $.qtip.prototype.toggle.calls.reset();
-          test.form.securityBeaconContainer().hide();
-          $(window).trigger('resize');
-          return Expect.waitForSpyCall($.qtip.prototype.toggle, test);
-        })
-        .then(function(test) {
-          expect($.qtip.prototype.toggle.calls.argsFor(0)).toEqual(jasmine.objectContaining({ 0: false }));
-          $.qtip.prototype.toggle.calls.reset();
-          test.form.securityBeaconContainer().show();
-          $(window).trigger('resize');
-          return Expect.waitForSpyCall($.qtip.prototype.toggle, test);
-        })
-        .then(function() {
-          expect($.qtip.prototype.toggle.calls.argsFor(0)).toEqual(jasmine.objectContaining({ 0: true }));
-        });
-    });


Moved into https://github.com/okta/okta-signin-widget/pull/3418/files#diff-7fc65ff7a9deb6e52de14ecbe846e256708dd5e763b6a70031bd7cbe0f57a5b7R120-R226

shawyu-okta · 2023-10-30T22:00:31Z

test/unit/spec/v1/EnrollCall_spec.js

    testEnrollPhoneNumber(setup, resEnrollSuccess);
  });

  describe('Verify phone number', function() {
+    beforeEach(() => {
+      jest.setTimeout(20000);
+    });


Checking also to see if these extra timeouts can be removed

Yep these can be removed, pushing update and did a search for all usage of this settimeout to test the removal of those as well 🙏

shawyu-okta · 2023-10-30T22:45:25Z

test/unit/helpers/dom/Dom.js

-
+  // jsdom has issue with :visible selector
+  // check visibility recursively instead
+  if (global.useJest) {


can we remove this conditional and the "jquery method" now? i see the jest setup file sets this true

glenfannin-okta mentioned this pull request Oct 17, 2023

OKTA-656562 : Complete migration of karma to jest #3413

Closed

5 tasks

glenfannin-okta marked this pull request as ready for review October 20, 2023 18:00

glenfannin-okta requested review from denysoblohin-okta, jaredperreault-okta, leemchale-okta, lesterchoi-okta, robertdamphousse-okta, shawyu-okta, shuowu-okta, trevoring-okta and vijetmahabaleshwar-okta as code owners October 20, 2023 18:00

glenfannin-okta force-pushed the OKTA-656562-karma-to-jest branch from 0e74b1b to c96c8da Compare October 20, 2023 22:10

shawyu-okta reviewed Oct 23, 2023

View reviewed changes

.testcaferc.js Outdated Show resolved Hide resolved

shawyu-okta reviewed Oct 23, 2023

View reviewed changes

glenfannin-okta requested a review from shawyu-okta October 25, 2023 01:12

This was referenced Oct 25, 2023

migrate karma to jest #3228

Closed

Spike: migrate okta-signin-widget Karma tests to Jest #3250

Closed

shawyu-okta reviewed Oct 26, 2023

View reviewed changes

glenfannin-okta commented Oct 26, 2023

View reviewed changes

glenfannin-okta force-pushed the OKTA-656562-karma-to-jest branch from d2a5958 to 5e8b814 Compare October 27, 2023 16:46

glenfannin-okta requested a review from shawyu-okta October 27, 2023 17:53

shawyu-okta approved these changes Oct 30, 2023

View reviewed changes

shuowu added 7 commits October 31, 2023 11:13

migrate AdminConsentRequired_spec.js

8ed9390

migrate EnrollPassword_spec.js

e943ac5

add global useJest variable

94146b9

add global variable for karma

cc45362

handle dom visibility issue in jest

be6478b

handle css in js dom and mock date in jest

bb4303c

add css build pretask to jest

c4b90b6

glenfannin-okta added 25 commits October 31, 2023 11:13

Fixing gen 1 testcafe specs

e47acbc

Updating assertion for primary auth spec

7390f53

Testing using device flow spec mock changes

0eb81cc

adding render: false param to navigateToPage func

d39b76c

Testing adding wait to primary auth spec

e2b3f8a

Fixing using device code flow spec

21cd436

removing only from test

4d915c8

Additional debugging for using device flow spec

86c3577

removing router feature flag from spec

65e4992

set features.router to false for primary auth spec

f1a51e6

Re-enabling devicecode activate testcafe spec

fc41927

Minor spec updates

18b33ab

Debugging usingdeviceflow spec

d9d90e3

Fixing device code and primary auth specs

0b3403d

Migrating loginRouter tests to testcafe

9b11d73

Removing dup entry in lockfile

e9dbfc5

Adding mock response for verify endpoint

67d3602

Enabling test to try on bacon

d30dbae

disabling failing test

b65387a

Fixing loginrouter spec and using fixturepath for gen1 determination

4c08648

Mocking getWithPopup auth-js function to avoid token verification logic

821df8e

PR Feedback updates

172dda9

Using spyOn from jest

47ec9d8

Updating yarn lock file

1c5e33d

PR feedback updates

fc0d5cf

glenfannin-okta force-pushed the OKTA-656562-karma-to-jest branch from 1423e45 to fc0d5cf Compare October 31, 2023 17:37

Re-adding Dom.isVisible logic

5a84cac

shawyu-okta approved these changes Oct 31, 2023

View reviewed changes

eng-services-aperture-github-bot-okta merged commit 6678e37 into master Oct 31, 2023

eng-services-aperture-github-bot-okta deleted the OKTA-656562-karma-to-jest branch October 31, 2023 23:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OKTA-656562 karma to jest #3418

OKTA-656562 karma to jest #3418

glenfannin-okta commented Oct 16, 2023 •

edited

shawyu-okta Oct 23, 2023

glenfannin-okta Oct 23, 2023

shawyu-okta Oct 25, 2023

glenfannin-okta Oct 27, 2023

shawyu-okta Oct 26, 2023

glenfannin-okta Oct 26, 2023

shawyu-okta Oct 26, 2023

glenfannin-okta Oct 27, 2023

shawyu-okta Oct 26, 2023

glenfannin-okta Oct 27, 2023

shawyu-okta Oct 26, 2023

glenfannin-okta Oct 27, 2023

glenfannin-okta Oct 26, 2023

glenfannin-okta Oct 26, 2023

shawyu-okta Oct 30, 2023

glenfannin-okta Oct 31, 2023

shawyu-okta Oct 30, 2023

OKTA-656562 karma to jest #3418

OKTA-656562 karma to jest #3418

Conversation

glenfannin-okta commented Oct 16, 2023 • edited

Description:

PR Checklist

Issue:

Reviewers:

Screenshot/Video:

Downstream Monolith Build:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glenfannin-okta commented Oct 16, 2023 •

edited