Bug 1880118: Improve Cypress `checkErrors` output #6475

dtaylor113 · 2020-08-28T20:46:46Z

This PR updates our usage of window.windowError in app.js and also improves Cypress checkErrors() output:

2) Monitoring: Alerts
       "after each" hook for "creates and expires a Silence":
     window error detected: {
 "message": "y is not defined",
 "stack": "ReferenceError: y is not defined
    at http://localhost:9000/static/main-e0abc76e4ffb6fa60c4c.js:95489:9
    at renderWithHooks (http://localhost:9000/static/vendors~main-2e679479202da74be6e9.js:371358:18)
    at mountIndeterminateComponent (http://localhost:9000/static/vendors~main-2e679479202da74be6e9.js:373592:13)
    at beginWork$1 (http://localhost:9000/static/vendors~main-2e679479202da74be6e9.js:374736:16)
    at HTMLUnknownElement.callCallback (http://localhost:9000/static/vendors~main-2e679479202da74be6e9.js:356597:14)
    at Object.invokeGuardedCallbackDev (http://localhost:9000/static/vendors~main-2e679479202da74be6e9.js:356647:16)
    at invokeGuardedCallback (http://localhost:9000/static/vendors~main-2e679479202da74be6e9.js:356704:31)
    at beginWork$$1 (http://localhost:9000/static/vendors~main-2e679479202da74be6e9.js:379467:7)
    at performUnitOfWork (http://localhost:9000/static/vendors~main-2e679479202da74be6e9.js:378458:12)
    at workLoopSync (http://localhost:9000/static/vendors~main-2e679479202da74be6e9.js:378435:22)"

frontend/packages/integration-tests-cypress/support/index.ts

frontend/packages/integration-tests-cypress/tests/crud/k8-openshift-cruds.spec.ts

frontend/public/components/app.jsx

frontend/packages/integration-tests-cypress/support/index.ts

frontend/public/declarations.d.ts

dtaylor113 · 2020-09-01T14:30:44Z

/retest

dtaylor113 · 2020-09-15T13:01:36Z

Hi @spadgett, PR paired down to just update window.windowError

spadgett · 2020-09-16T14:11:29Z

frontend/packages/integration-tests-cypress/support/index.ts

@@ -59,7 +59,13 @@ Cypress.Commands.add('testA11y', (target: string) => {
 export const checkErrors = () =>
  cy.window().then((win) => {
    if (win.windowError) {
-      throw new Error(`window/js runtime error detected: ${win.windowError}`);
+      throw new Error(


Does throw win.windowError work instead of creating a new Error? Or adding an assertion that windowError is nullish?

Hmm...I had tried it in the past and it didn't work, but I haven't tried it since I truley changed win.windowError to be an Error not an Event. I'll try to throw it again now that it's a true Error

throw win.windowError results in the following; we lose stack-trace, but since it's minified, not sure how useful it is:

ReferenceError: i is not defined

As opposed to throwing new Error w/ JSON.stringify & \n processing:

window error detected: { "message": "i is not defined", "stack": "ReferenceError: i is not defined at http://localhost:9000/static/main-35bf7a55e8c0461351e2.js:102081:5 at renderWithHooks (http://localhost:9000/static/vendors~main-47b21d89223a2a918b47.js:371358:18) at mountIndeterminateComponent (http://localhost:9000/static/vendors~main-47b21d89223a2a918b47.js:373592:13) at beginWork$1 (http://localhost:9000/static/vendors~main-47b21d89223a2a918b47.js:374736:16) at HTMLUnknownElement.callCallback (http://localhost:9000/static/vendors~main-47b21d89223a2a918b47.js:356597:14) at Object.invokeGuardedCallbackDev (http://localhost:9000/static/vendors~main-47b21d89223a2a918b47.js:356647:16) at invokeGuardedCallback (http://localhost:9000/static/vendors~main-47b21d89223a2a918b47.js:356704:31) at beginWork$$1 (http://localhost:9000/static/vendors~main-47b21d89223a2a918b47.js:379467:7) at performUnitOfWork (http://localhost:9000/static/vendors~main-47b21d89223a2a918b47.js:378458:12) at workLoopSync (http://localhost:9000/static/vendors~main-47b21d89223a2a918b47.js:378435:22)" }

spadgett · 2020-09-16T14:14:31Z

frontend/public/components/app.jsx

+  window.windowError = error;
+};
+window.onunhandledrejection = (promiseRejectionEvent) => {
+  const error = new Error(`unhandled promise rejection: ${promiseRejectionEvent.reason}`);


If this isn't already an error, maybe we just leave window.windowError as Error | Event and just check that it's not defined in Cypress rather than trying to rethrow it.

I don't think we'd see any output for promiseRejectionEvent in Cypress if we did that.

Note: previously the window.windowError was either an ErrorEvent or promiseRejectionEvent, it never was an Error. I had to change the method signiture params in order to get the Error: window.onerror = (message, source, lineno, colno, --->error<----).

I do know that when window.windowError was a promiseRejectionEvent, throwing it didn't produce any output in headless. I believe it has to be an Error. I believe I even tried doing window.dispatch(..) when it was a promiseRejectionEvent, but it didn't produce any output.

Right, you wouldn't throw the event. You would check that it's not null and log the content (if not already logged by Cypress).

idk, I feel casting all into an Error and just handling/throwing the Error (directly or recreating w/ formatting), to me seems more straight forward.

Unless you are saying we should log the promiseRejectionEvent, not throw an Error? So that the cypress tests won't fail on a promiseRejectionEvent?

I'm saying we have an assertion that causes the test to fail if windowError is never null. Something like

expect(win.windowError).to.not.exist;

Hi, tried

assert.isNull( win.windowError, `${(win.windowError as PromiseRejectionEvent).reason || (win.windowError as Error).message} ${ (win.windowError as Error).stack }`

Which, in addition to the message (2nd arg in assert(..)), also output the tested object. Unfortunately, since window is part of the PromiseRejectionEvent, it gets spit out along with a lot of other attributes:

2) Monitoring: Alerts "after each" hook for "creates and expires a Silence": path /silence_WRONG_URL/63646b85-b4a6-4ead-8a35-609a77317edf was not found undefined: expected { isTrusted: [Getter], promise: {}, reason: 'path /silence_WRONG_URL/63646b85-b4a6-4ead-8a35-609a77317edf was not found', NONE: 0, CAPTURING_PHASE: 1, AT_TARGET: 2, BUBBLING_PHASE: 3, type: 'unhandledrejection', target: { parent: { parent: [Circular], opener: null, top: [Circular], length: 2, frames: [Circular], closed: false, location: [Object], self: [Circular], window: [Circular], document: [Getter], name: [Getter/Setter], customElements: [Getter], history: [Getter], locationbar: [Getter/Setter], menubar: [Getter/Setter], personalbar: [Getter/Setter], scrollbars: [Getter/Setter], statusbar: [Getter/Setter], toolbar: [Getter/Setter], status: [Getter/Setter], frameElement: [Getter], navigator: [Getter], origin: [Getter/Setter], external: [Getter/Setter], screen: [Getter/Setter], innerWidth: [Getter/Setter], innerHeight: [Getter/Setter], scrollX: [Getter/Setter], pageXOffset: [Getter/Setter], scrollY: [Getter/Setter], pageYOffset: [Getter/Setter], visualViewport: [Getter/Setter], screenX: [Getter/Setter], screenY: [Getter/Setter], outerWidth: [Getter/Setter], outerHeight: [Getter/Setter], devicePixelRatio: [Getter/Setter], clientInformation: [Getter/Setter], ... ... -- additional two screens worth of output ---

Latest code I think moves closer to what your after. Changed the assertion so it doesn't ouput entire contents of win.windowError. It does output ...: expected false to be true which doesn't add any value:

assert.isTrue( win.windowError === null, `window error detected: ${(win.windowError as PromiseRejectionEvent).reason || (win.windowError as Error).message} ${ (win.windowError as Error).stack ? (win.windowError as Error).stack.replace(/\\n/g, '\n') : '' }`, );

Produces:

...when windowError is a PromiseRejectionEvent: 2) Monitoring: Alerts "after each" hook for "creates and expires a Silence": AssertionError: window error detected: path /silence_WRONG_URL/b0ef08c1-e09c-41ab-a046-761d47a71bee was not found : expected false to be true ...when windowError is an Error: 2) Monitoring: Alerts "after each" hook for "creates and expires a Silence": window error detected: i is not defined ReferenceError: i is not defined at http://localhost:9000/static/main-66d801b01502734ad2c0.js:96412:5 at renderWithHooks (http://localhost:9000/static/vendors~main-9d6cea73c651a7e7c901.js:377121:18) at mountIndeterminateComponent (http://localhost:9000/static/vendors~main-9d6cea73c651a7e7c901.js:379355:13) at beginWork$1 (http://localhost:9000/static/vendors~main-9d6cea73c651a7e7c901.js:380499:16) at HTMLUnknownElement.callCallback (http://localhost:9000/static/vendors~main-9d6cea73c651a7e7c901.js:362360:14) at Object.invokeGuardedCallbackDev (http://localhost:9000/static/vendors~main-9d6cea73c651a7e7c901.js:362410:16) at invokeGuardedCallback (http://localhost:9000/static/vendors~main-9d6cea73c651a7e7c901.js:362467:31) at beginWork$$1 (http://localhost:9000/static/vendors~main-9d6cea73c651a7e7c901.js:385230:7) at performUnitOfWork (http://localhost:9000/static/vendors~main-9d6cea73c651a7e7c901.js:384221:12) at workLoopSync (http://localhost:9000/static/vendors~main-9d6cea73c651a7e7c901.js:384198:22): expected false to be true

The only way to generate output in headless in afterEach is either throwing an Error or failing an assertion. cy.task('log',' ...) which uses console.log doesn't output anything :-(

openshift-ci-robot · 2020-09-17T17:45:17Z

@dtaylor113: This pull request references Bugzilla bug 1880118, which is valid. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1880118: Improve Cypress checkErrors output

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

dtaylor113 · 2020-09-22T15:50:41Z

/retest

frontend/public/components/app.jsx

spadgett · 2020-09-23T17:10:25Z

frontend/packages/integration-tests-cypress/support/index.ts

+          }`
+        : 'no window error detected';
+
+    assert.isTrue(win.windowError === null, assertMsg);


Don't check specifically for null. You want to handle undefined as well.

spadgett · 2020-09-23T17:14:53Z

frontend/packages/integration-tests-cypress/support/index.ts

+        ? `window error detected: ${(win.windowError as PromiseRejectionEvent).reason ||
+            (win.windowError as Error).message} ${
+            (win.windowError as Error).stack
+              ? (win.windowError as Error).stack.replace(/\\n/g, '\n')
+              : ''
+          }`


This is too much on one line. I'd pull this out into a helper. Something like

const formatWindowError = (windowError: Error | PromiseRejectionEvent) => { const { reason } = windowError as PromiseRejectionEvent; if (reason) { return reason; } const { message, stack } = windowError as Error; const formattedStack = stack?.replace(/\\n/g, '\n'); return `${message} ${formattedStack}`; };

thanks, fixed.

spadgett · 2020-09-23T17:15:12Z

frontend/packages/integration-tests-cypress/support/index.ts

-      throw new Error(`window/js runtime error detected: ${win.windowError}`);
-    }
+    const assertMsg =
+      win.windowError !== null


Handle undefined as well.

Suggested change

win.windowError !== null

win.windowError

spadgett

/lgtm

openshift-ci-robot · 2020-09-23T19:10:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dtaylor113, spadgett

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [spadgett]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2020-09-23T19:48:26Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-09-23T20:14:29Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2020-09-23T22:45:43Z

@dtaylor113: All pull requests linked via external trackers have merged:

Bugzilla bug 1880118 has been moved to the MODIFIED state.

In response to this:

Bug 1880118: Improve Cypress checkErrors output

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

dtaylor113 requested a review from spadgett August 28, 2020 20:46

openshift-ci-robot requested review from jeff-phillips-18 and kyoto August 28, 2020 20:47

spadgett reviewed Aug 28, 2020

View reviewed changes

frontend/packages/integration-tests-cypress/support/index.ts Outdated Show resolved Hide resolved

frontend/packages/integration-tests-cypress/support/index.ts Show resolved Hide resolved

dtaylor113 force-pushed the cypress-check-error-fix branch from e3d7209 to b23364a Compare August 28, 2020 21:09

openshift-ci-robot added the component/core Related to console core functionality label Aug 28, 2020

dtaylor113 force-pushed the cypress-check-error-fix branch from b23364a to 14a346d Compare August 28, 2020 21:11

spadgett reviewed Aug 28, 2020

View reviewed changes

frontend/packages/integration-tests-cypress/support/index.ts Outdated Show resolved Hide resolved

dtaylor113 force-pushed the cypress-check-error-fix branch from 14a346d to 02f1cf6 Compare August 31, 2020 19:06

spadgett reviewed Aug 31, 2020

View reviewed changes

frontend/packages/integration-tests-cypress/tests/crud/k8-openshift-cruds.spec.ts Outdated Show resolved Hide resolved

frontend/public/components/app.jsx Outdated Show resolved Hide resolved

dtaylor113 force-pushed the cypress-check-error-fix branch 2 times, most recently from c61c836 to e5b8b6e Compare August 31, 2020 21:27

spadgett reviewed Sep 1, 2020

View reviewed changes

frontend/packages/integration-tests-cypress/support/index.ts Outdated Show resolved Hide resolved

spadgett reviewed Sep 1, 2020

View reviewed changes

frontend/public/declarations.d.ts Outdated Show resolved Hide resolved

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 8, 2020

dtaylor113 changed the title ~~Improve Cypress checkErrors output~~ [WIP] Improve Cypress checkErrors output Sep 9, 2020

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 9, 2020

dtaylor113 force-pushed the cypress-check-error-fix branch from e5b8b6e to 5b65430 Compare September 10, 2020 20:47

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 10, 2020

dtaylor113 force-pushed the cypress-check-error-fix branch from 5b65430 to 783e4e3 Compare September 15, 2020 12:59

dtaylor113 changed the title ~~[WIP] Improve Cypress checkErrors output~~ Improve Cypress checkErrors output Sep 15, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 15, 2020

spadgett reviewed Sep 16, 2020

View reviewed changes

dtaylor113 changed the title ~~Improve Cypress checkErrors output~~ Bug 1880118: Improve Cypress checkErrors output Sep 17, 2020

openshift-ci-robot added bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Sep 17, 2020

dtaylor113 force-pushed the cypress-check-error-fix branch from 783e4e3 to a477405 Compare September 18, 2020 17:41

openshift deleted a comment from openshift-ci-robot Sep 18, 2020

openshift deleted a comment from openshift-ci-robot Sep 21, 2020

openshift deleted a comment from openshift-ci-robot Sep 22, 2020

dtaylor113 force-pushed the cypress-check-error-fix branch from a477405 to 8428b40 Compare September 22, 2020 20:52

openshift-ci-robot mentioned this pull request Sep 23, 2020

Bug 1880118: Cypress a11y enhancements #6698

Merged

dtaylor113 force-pushed the cypress-check-error-fix branch from 8428b40 to cdbda17 Compare September 23, 2020 13:26

spadgett reviewed Sep 23, 2020

View reviewed changes

updated window.windowError and improved Cypress checkErrors output

2c4b596

dtaylor113 force-pushed the cypress-check-error-fix branch from cdbda17 to 2c4b596 Compare September 23, 2020 17:56

spadgett approved these changes Sep 23, 2020

View reviewed changes

openshift-ci-robot assigned spadgett Sep 23, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 23, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 23, 2020

openshift-merge-robot merged commit 8f325f2 into openshift:master Sep 23, 2020

spadgett added this to the v4.6 milestone Sep 30, 2020

dtaylor113 deleted the cypress-check-error-fix branch November 25, 2020 14:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1880118: Improve Cypress `checkErrors` output #6475

Bug 1880118: Improve Cypress `checkErrors` output #6475

dtaylor113 commented Aug 28, 2020 •

edited

Loading

dtaylor113 commented Sep 1, 2020

dtaylor113 commented Sep 15, 2020

spadgett Sep 16, 2020

dtaylor113 Sep 16, 2020 •

edited

Loading

dtaylor113 Sep 16, 2020

spadgett Sep 16, 2020

dtaylor113 Sep 16, 2020

dtaylor113 Sep 16, 2020 •

edited

Loading

dtaylor113 Sep 16, 2020 •

edited

Loading

spadgett Sep 16, 2020

dtaylor113 Sep 17, 2020 •

edited

Loading

spadgett Sep 17, 2020

dtaylor113 Sep 18, 2020

dtaylor113 Sep 18, 2020

openshift-ci-robot commented Sep 17, 2020

dtaylor113 commented Sep 22, 2020

spadgett Sep 23, 2020

dtaylor113 Sep 23, 2020

spadgett Sep 23, 2020

dtaylor113 Sep 23, 2020

spadgett Sep 23, 2020

dtaylor113 Sep 23, 2020

spadgett left a comment

openshift-ci-robot commented Sep 23, 2020

openshift-bot commented Sep 23, 2020

openshift-bot commented Sep 23, 2020

openshift-ci-robot commented Sep 23, 2020

Bug 1880118: Improve Cypress checkErrors output #6475

Bug 1880118: Improve Cypress checkErrors output #6475

Conversation

dtaylor113 commented Aug 28, 2020 • edited Loading

dtaylor113 commented Sep 1, 2020

dtaylor113 commented Sep 15, 2020

Choose a reason for hiding this comment

dtaylor113 Sep 16, 2020 • edited Loading

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

dtaylor113 Sep 16, 2020 • edited Loading

Choose a reason for hiding this comment

dtaylor113 Sep 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtaylor113 Sep 17, 2020 • edited Loading

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

openshift-ci-robot commented Sep 17, 2020

dtaylor113 commented Sep 22, 2020

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

spadgett left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Sep 23, 2020

openshift-bot commented Sep 23, 2020

openshift-bot commented Sep 23, 2020

openshift-ci-robot commented Sep 23, 2020

Bug 1880118: Improve Cypress `checkErrors` output #6475

Bug 1880118: Improve Cypress `checkErrors` output #6475

dtaylor113 commented Aug 28, 2020 •

edited

Loading

dtaylor113 Sep 16, 2020 •

edited

Loading

dtaylor113 Sep 16, 2020 •

edited

Loading

dtaylor113 Sep 16, 2020 •

edited

Loading

dtaylor113 Sep 17, 2020 •

edited

Loading