third_party/WebKit/LayoutTests/custom-elements/imports/imports-create.html - Issue 2242743002: Make custom elements work in HTML imports

Side by Side Diff: third_party/WebKit/LayoutTests/custom-elements/imports/imports-create.html

Issue 2242743002: Make custom elements work in HTML imports (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: rebase wip Created 4 years, 4 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« third_party/WebKit/LayoutTests/custom-elements/imports/circular-imports.html ('K') | « third_party/WebKit/LayoutTests/custom-elements/imports/create-element-in-import.html ('k') | third_party/WebKit/LayoutTests/custom-elements/imports/imports-define-in-import.html » ('j') | third_party/WebKit/LayoutTests/custom-elements/imports/parser-customize-imports.html » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>
	dominicc (has gone to gerrit) 2016/08/19 04:54:11 I'm not sure from the name what this test is about I'm not sure from the name what this test is about. Dynamically created imports? kochi 2016/08/19 10:51:47 The counterpart of this test is imports-upgrade.ht Show quoted text On 2016/08/19 04:54:11, dominicc wrote: > I'm not sure from the name what this test is about. Dynamically created imports? The counterpart of this test is imports-upgrade.html. This tests when custom elements constructor called when "created" (by parser), while the other tests when custom elements constructor called when "upgraded" (via queued callback). Will rename. kochi 2016/08/22 07:37:34 Renamed to imports-and-synchronous-crate.html. Show quoted text On 2016/08/19 10:51:47, kochi wrote: > On 2016/08/19 04:54:11, dominicc wrote: > > I'm not sure from the name what this test is about. Dynamically created > imports? > > The counterpart of this test is imports-upgrade.html. > This tests when custom elements constructor called when "created" (by parser), > while the other tests when custom elements constructor called when "upgraded" > (via queued callback). > > Will rename. Renamed to imports-and-synchronous-crate.html.
	2 <script src="../../resources/testharness.js"></script>

	3 <script src="../../resources/testharnessreport.js"></script>

	4 <script>

	5 'use strict';

	6 setup({ explicit_done: true });
	dominicc (has gone to gerrit) 2016/08/19 04:54:10 When there are multiple tests in a file it would b When there are multiple tests in a file it would be neater to use test_in_window or promise_test to do it. Maybe you could write something which resolves a promise when the import loads; it should make your tests quite neat.
	7 let reactions = [];

	8 customElements.define('x-x', class extends HTMLElement {

	9 constructor() {

	10 super();

	11 reactions.push({ type: 'constructor', element: this });

	12 }

	13 });

	14 test(() => {

	15 assert_equals(reactions.length, 0);

	16 }, 'Should not have parsed <x-x> yet');

	17 </script>

	18

	19 <x-x></x-x>

	20

	21 <script>

	22 test(() => {

	23 assert_equals(reactions.length, 1);
	dominicc (has gone to gerrit) 2016/08/19 04:54:11 I think it is better to keep related assertions to I think it is better to keep related assertions together in one test. You can use a promise_test or async_test if you want to split the test body over multiple script tags.
	24 }, 'Parser should invoke the custom element constructor');

	25

	26 let import1 = document.createElement('link');

	27 import1.rel = 'import';

	28 import1.href = 'resources/import-custom.html';

	29 document.head.appendChild(import1);

	30

	31 import1.onload = () => {

	32 test(() => {

	33 assert_equals(reactions.length, 2);

	34 }, 'import should invoke the custom element constructor');
	dominicc (has gone to gerrit) 2016/08/19 04:54:11 It's a bit hard to keep track of what's going on a It's a bit hard to keep track of what's going on across these multiple files. It would be clearer if: - Before something happens, assert_array_equals(thing, []) as a sanity check. - Run the thing/wait for the condition. - Dig up the elements of interest with query selector. - assert_array_equals(thing, [a, b]) Then be reading one block of code I can see what's being asserted and what should have come from where. In particular, just asserting the length is two here doesn't tell me what the entries are or what order they're in. We should know that, right?
	35

	36 let elementsInMaster = document.getElementsByTagName('x-x');

	37 let elementsInImport = import1.import.getElementsByTagName('x-x');

	38 test(() => {

	39 assert_equals(reactions[0].element, elementsInMaster[0]);

	40 assert_equals(reactions[1].element, elementsInImport[0]);

	41 }, 'Constructor should run in the order elements are created');
	dominicc (has gone to gerrit) 2016/08/19 04:54:11 So... per the above comment... the test would be m So... per the above comment... the test would be more readable if it was one assert_array_equals assertion that had these elements in order. I don't see why those would be split across multiple tests, either. Having the right number, order and values in a sequence of calls seems all related.
	42

	43 done();

	44 };

	45 </script>

OLD	NEW