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.

Sign up for GitHub

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

Jump to bottom

[testharness.js] Disallow test return value #12958

Merged
merged 4 commits into from Sep 17, 2018
Merged

[testharness.js] Disallow test return value #12958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
256eb57
[tesrtharness.js] Disallow `test` return value
jugglinmike Sep 11, 2018
1132ca1
Report invalid test with harness error
jugglinmike Sep 13, 2018
46cabd4
fixup! Report invalid test with harness error
jugglinmike Sep 13, 2018
04e43a4
fixup! Report invalid test with harness error
jugglinmike Sep 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
155 changes: 155 additions & 0 deletions resources/test/tests/unit/test-return-restrictions.html
View file
Open in desktop
@@ -0,0 +1,155 @@
<!DOCTYPE HTML>
<html>
<head>
<meta charset="utf-8">
<script src="/resources/testharness.js"></script>
<title>Restrictions on return value from `test`</title>
</head>
<body>
<script>
function makeTest(...bodies) {
const closeScript = '<' + '/script>';
let src = `
<!DOCTYPE HTML>
<html>
<head>
<title>Document title</title>
<script src="/resources/testharness.js?${Math.random()}">${closeScript}
</head>

<body>
<div id="log"></div>`;
bodies.forEach((body) => {
src += '<script>(' + body + ')();' + closeScript;
});

const iframe = document.createElement('iframe');

document.body.appendChild(iframe);
iframe.contentDocument.write(src);

return new Promise((resolve) => {
window.addEventListener('message', function onMessage(e) {
if (e.source !== iframe.contentWindow) {
return;
}
if (!e.data || e.data.type !=='complete') {
return;
}
window.removeEventListener('message', onMessage);
resolve(e.data);
});

iframe.contentDocument.close();
}).then(({ tests, status }) => {
const summary = {
harness: {
status: getEnumProp(status, status.status),
message: status.message
},
tests: {}
};

tests.forEach((test) => {
summary.tests[test.name] = getEnumProp(test, test.status);
});

return summary;
});
}

function getEnumProp(object, value) {
for (let property in object) {
if (!/^[A-Z]+$/.test(property)) {
continue;
}

if (object[property] === value) {
return property;
}
}
}

promise_test(() => {
return makeTest(
() => {
test(() => undefined, 'before');
test(() => null, 'null');
test(() => undefined, 'after');
}
).then(({harness, tests}) => {
assert_equals(harness.status, 'ERROR');
assert_equals(
harness.message,
'test named "null" inappropriately returned a value'
);
assert_equals(tests.before, 'PASS');
assert_equals(tests.null, 'PASS');
assert_equals(tests.after, 'PASS');
});
}, 'test returning `null`');

promise_test(() => {
return makeTest(
() => {
test(() => undefined, 'before');
test(() => ({}), 'object');
test(() => undefined, 'after');
}
).then(({harness, tests}) => {
assert_equals(harness.status, 'ERROR');
assert_equals(
harness.message,
'test named "object" inappropriately returned a value'
);
assert_equals(tests.before, 'PASS');
assert_equals(tests.object, 'PASS');
assert_equals(tests.after, 'PASS');
});
}, 'test returning an ordinary object');

promise_test(() => {
return makeTest(
() => {
test(() => undefined, 'before');
test(() => ({ then() {} }), 'thenable');
test(() => undefined, 'after');
}
).then(({harness, tests}) => {
assert_equals(harness.status, 'ERROR');
assert_equals(
harness.message,
'test named "thenable" inappropriately returned a value, consider using `promise_test` instead'
);
assert_equals(tests.before, 'PASS');
assert_equals(tests.thenable, 'PASS');
assert_equals(tests.after, 'PASS');
});
}, 'test returning a thenable object');

promise_test(() => {
return makeTest(
() => {
test(() => undefined, 'before');
test(() => {
const iframe = document.createElement('iframe');
iframe.setAttribute('sandbox', '');
document.body.appendChild(iframe);
return iframe.contentWindow;
}, 'restricted');
test(() => undefined, 'after');
}
).then(({harness, tests}) => {
assert_equals(harness.status, 'ERROR');
assert_equals(
harness.message,
'test named "restricted" inappropriately returned a value'
);
assert_equals(tests.before, 'PASS');
assert_equals(tests.restricted, 'PASS');
assert_equals(tests.after, 'PASS');
});
}, 'test returning a restricted object');
</script>
</body>
</html>
17 changes: 16 additions & 1 deletion resources/testharness.js
View file
Open in desktop
Expand Up @@ -541,7 +541,22 @@ policies and contribution forms [3].
var test_name = name ? name : test_environment.next_default_test_name();
properties = properties ? properties : {};
var test_obj = new Test(test_name, properties);
test_obj.step(func, test_obj, test_obj);
var value = test_obj.step(func, test_obj, test_obj);

if (value !== undefined) {
var msg = "test named \"" + test_name +
"\" inappropriately returned a value";

try {
if (value && value.hasOwnProperty("then")) {
msg += ", consider using `promise_test` instead";
}
} catch (err) {}

tests.status.status = tests.status.ERROR;
tests.status.message = msg;
}

if (test_obj.phase === test_obj.phases.STARTED) {
test_obj.done();
}
Expand Down
2 changes: 1 addition & 1 deletion web-animations/timing-model/animations/setting-the-target-effect-of-an-animation.html
View file
Open in desktop
Expand Up @@ -113,7 +113,7 @@
'existing animation, the target effect\'s timing is updated to reflect ' +
'the current time of the new animation.');

test(async t => {
promise_test(async t => {
const anim = createDiv(t).animate(null, 100 * MS_PER_SEC);
anim.updatePlaybackRate(2);
assert_equals(anim.playbackRate, 1);
Expand Down