CI: event-handler-dynamic fails with exceptions #4087

ghost · 2019-12-10T19:00:05Z

Describe the bug
CI logs show event-handler-dynamic failing to run with exceptions thrown after triggering button click events.

Logs

2019-12-10T16:53:34.3087021Z Error: Uncaught [TypeError: Cannot read property 'apply' of undefined]
2019-12-10T16:53:34.3088149Z     at reportException (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/helpers/runtime-script-errors.js:62:24)
2019-12-10T16:53:34.3089015Z     at innerInvokeEventListeners (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:332:9)
2019-12-10T16:53:34.3089824Z     at invokeEventListeners (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:267:3)
2019-12-10T16:53:34.3090633Z     at HTMLButtonElementImpl._dispatch (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:214:9)
2019-12-10T16:53:34.3091430Z     at HTMLButtonElementImpl.dispatchEvent (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:87:17)
2019-12-10T16:53:34.3091937Z     at HTMLButtonElement.dispatchEvent (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/generated/EventTarget.js:144:23)
2019-12-10T16:53:34.3092817Z     at Object.test (/home/runner/work/svelte/svelte/test/runtime/samples/event-handler-dynamic/_config.js:18:16)
2019-12-10T16:53:34.3093079Z     at Promise.resolve.then (/home/runner/work/svelte/svelte/test/runtime/index.js:193:37)
2019-12-10T16:53:34.3093490Z     at <anonymous> TypeError: Cannot read property 'apply' of undefined
2019-12-10T16:53:34.3094035Z     at HTMLButtonElement.<anonymous> (/home/runner/work/svelte/svelte/test/runtime/samples/event-handler-dynamic/main.svelte:37:30)
2019-12-10T16:53:34.3094543Z     at innerInvokeEventListeners (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:316:27)
2019-12-10T16:53:34.3095027Z     at invokeEventListeners (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:267:3)
2019-12-10T16:53:34.3095507Z     at HTMLButtonElementImpl._dispatch (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:214:9)
2019-12-10T16:53:34.3096012Z     at HTMLButtonElementImpl.dispatchEvent (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:87:17)
2019-12-10T16:53:34.3096497Z     at HTMLButtonElement.dispatchEvent (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/generated/EventTarget.js:144:23)
2019-12-10T16:53:34.3097185Z     at Object.test (/home/runner/work/svelte/svelte/test/runtime/samples/event-handler-dynamic/_config.js:18:16)
2019-12-10T16:53:34.3097754Z     at Promise.resolve.then (/home/runner/work/svelte/svelte/test/runtime/index.js:193:37)
2019-12-10T16:53:34.3097902Z     at <anonymous>
2019-12-10T16:53:34.3154982Z     âœ“ event-handler-dynamic  (42ms)
2019-12-10T16:53:34.3450414Z Error: Uncaught [TypeError: Cannot read property 'apply' of undefined]
2019-12-10T16:53:34.3451353Z     at reportException (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/helpers/runtime-script-errors.js:62:24)
2019-12-10T16:53:34.3452328Z     at innerInvokeEventListeners (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:332:9)
2019-12-10T16:53:34.3453515Z     at invokeEventListeners (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:267:3)
2019-12-10T16:53:34.3455745Z     at HTMLButtonElementImpl._dispatch (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:214:9)
2019-12-10T16:53:34.3458053Z     at HTMLButtonElementImpl.dispatchEvent (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:87:17)
2019-12-10T16:53:34.3458987Z     at HTMLButtonElement.dispatchEvent (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/generated/EventTarget.js:144:23)
2019-12-10T16:53:34.3459817Z     at Object.test (/home/runner/work/svelte/svelte/test/runtime/samples/event-handler-dynamic/_config.js:18:16)
2019-12-10T16:53:34.3460313Z     at Promise.resolve.then (/home/runner/work/svelte/svelte/test/runtime/index.js:193:37)
2019-12-10T16:53:34.3461334Z     at <anonymous> TypeError: Cannot read property 'apply' of undefined
2019-12-10T16:53:34.3462291Z     at HTMLButtonElement.<anonymous> (/home/runner/work/svelte/svelte/test/runtime/samples/event-handler-dynamic/main.svelte:67:30)
2019-12-10T16:53:34.3463254Z     at innerInvokeEventListeners (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:316:27)
2019-12-10T16:53:34.3464028Z     at invokeEventListeners (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:267:3)
2019-12-10T16:53:34.3464808Z     at HTMLButtonElementImpl._dispatch (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:214:9)
2019-12-10T16:53:34.3465599Z     at HTMLButtonElementImpl.dispatchEvent (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:87:17)
2019-12-10T16:53:34.3466102Z     at HTMLButtonElement.dispatchEvent (/home/runner/work/svelte/svelte/node_modules/jsdom/lib/jsdom/living/generated/EventTarget.js:144:23)
2019-12-10T16:53:34.3466795Z     at Object.test (/home/runner/work/svelte/svelte/test/runtime/samples/event-handler-dynamic/_config.js:18:16)
2019-12-10T16:53:34.3467233Z     at Promise.resolve.then (/home/runner/work/svelte/svelte/test/runtime/index.js:193:37)

To Reproduce
Take a look at the CI logs

Expected behavior
The test case should run without exceptions or the CI check should fail.

Severity
Moderate.

If more tests fail in this manner, we'll be missing possible regressions because we think the CI check is OK when its not.

The text was updated successfully, but these errors were encountered:

ghost · 2019-12-10T19:04:09Z

@tanhauhau, mind taking a look at this?
I see you were the last committer to this file.

The offending file is here: event-handler-dynamic

Conduitry · 2019-12-10T19:07:28Z

It's passing for me locally on the latest master, and it's passing in github's CI - https://github.com/sveltejs/svelte/commit/0a6310f7a39b76bb884251cf3cc385c9298645db/checks?check_suite_id=351586083

ghost · 2019-12-10T19:10:33Z

@Conduitry, I pulled that exception from the latest master... the problem is that it is passing.
The test doesn't actually run because of the exception.... though you are right, Svelte does the right thing if you copy/paste the test case into a REPL.

False negatives are crazy bad for regression tests.

ghost · 2019-12-10T19:32:31Z

Performing the test steps manually in Chrome dev mode gives the same result:

Here's a REPL:
https://svelte.dev/repl/54f3229b24e0493c9f7faf0f2dae2dfc?version=3.16.3

ghost · 2019-12-10T19:48:16Z

I found the issue...

The test wants to see that clicking the button before assigning the handler does nothing.
Since the variable is unassigned, it throws the exception as it should.
If we really want to test dynamic assignment, we should give the handler a no-op function to start.

anlexN · 2019-12-11T06:53:06Z

i don't understand your statement:

let [hand1, hand2, btn] = div.querySelectorAll('button')

i have searched MDN, but there are no api like this.
and what do you want to do?

ghost · 2019-12-12T19:40:29Z

Closing this in favor of the root cause specified in #4090.

ghost · 2019-12-13T13:35:48Z

I've reopened this issue as a placeholder for the root cause of why exceptions in Svelte runtime don't fail tests. The actually answer lies in JSDOM's approximation of how real exceptions are handled by the browser.

In the specific test mentioned here, the event handler installed by Svelte has a bug where it doesn't check the validity of the callback before trying to execute it. Since the runtime is being triggered by dispatchEvent(click), the resulting exception gets trapped by JSDOM, barfed out to the logs, and then dropped on the floor.

The simple solution is to install an onerror handler on the window and preventDefault() to stop the log stack trace. From the handler, we can then do something meaningful. I've been doing this on a test-by-test basis, but for long-term, we should look at installing the handler at a higher level so that any future test will be automatically protected false negatives due to runtime exceptions.

With the handler installed, you get a much clearer (and failed) test output:

let err = "";
window.addEventListener('error', (e) => {
	e.preventDefault();
	err = e.message;
});

await button.dispatchEvent(event);
assert.equal(err, "", err);

  1) runtime
       event-handler-dynamic :

      AssertionError [ERR_ASSERTION]: Cannot read property 'apply' of undefined
      + expected - actual

      -Cannot read property 'apply' of undefined

      at Object.test (test\runtime\samples\event-handler-dynamic\_config.js:24:10)

  cmd-click: test\runtime\samples\event-handler-dynamic/main.svelte

This was referenced Dec 10, 2019

tests: event-handler-dynamic: add no-op handler #4088

Closed

on:click "Cannot read property 'apply' of undefined" #4090

Closed

ghost closed this as completed Dec 12, 2019

ghost reopened this Dec 13, 2019

ghost mentioned this issue Dec 13, 2019

compiler/tests: dynamic events: validate handler before executing #4105

Merged

4 tasks

Conduitry closed this as completed in #4105 Dec 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI: event-handler-dynamic fails with exceptions #4087

CI: event-handler-dynamic fails with exceptions #4087

ghost commented Dec 10, 2019

ghost commented Dec 10, 2019

Conduitry commented Dec 10, 2019

ghost commented Dec 10, 2019 •

edited by ghost

Loading

ghost commented Dec 10, 2019 •

edited by ghost

Loading

ghost commented Dec 10, 2019

anlexN commented Dec 11, 2019

ghost commented Dec 12, 2019

ghost commented Dec 13, 2019 •

edited by ghost

Loading

CI: event-handler-dynamic fails with exceptions #4087

CI: event-handler-dynamic fails with exceptions #4087

Comments

ghost commented Dec 10, 2019

ghost commented Dec 10, 2019

Conduitry commented Dec 10, 2019

ghost commented Dec 10, 2019 • edited by ghost Loading

ghost commented Dec 10, 2019 • edited by ghost Loading

ghost commented Dec 10, 2019

anlexN commented Dec 11, 2019

ghost commented Dec 12, 2019

ghost commented Dec 13, 2019 • edited by ghost Loading

ghost commented Dec 10, 2019 •

edited by ghost

Loading

ghost commented Dec 10, 2019 •

edited by ghost

Loading

ghost commented Dec 13, 2019 •

edited by ghost

Loading