Skip to content

Commit

Permalink
Address flakiness in import-css-module-basic.html
Browse files Browse the repository at this point in the history 
The logs from the failures are not helpful in diagnosing the issue,
and I'm unable to reproduce any failure locally.

My best guess is that the issue is related to the test's use of 5
iframes that are injected in async_tests. There's probably some
strange timing issue that happens if things happen to load in a
particular order.

This change attempts to address the flakiness by simplifying the test.
4 of the iframes are removed and the modules are just loaded in order
at the top level instead.

The 5th test remains in an iframe because the test is supposed to
cause an error; this one is converted to promise_test.

Bug: 1225913
Change-Id: If1870af360ba6812689c03e87ce4d21d91faabe1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3003700
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#898417}
  • Loading branch information
@dandclark @chromium-wpt-export-bot
dandclark authored and chromium-wpt-export-bot committed Jul 3, 2021
1 parent aebba15 commit 07a56dc
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 131 deletions.
113 changes: 58 additions & 55 deletions html/semantics/scripting-1/the-script-element/css-module/import-css-module-basic.html
View file
Original file line number Diff line number Diff line change
@@ -1,80 +1,83 @@
<!doctype html>

<head>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
</head>

<body>
<div id="test">I am a test div.</div>
<div id="test2">I am a test div.</div>
<div id="test3">I am a test div.</div>
<div id="test3b">I am a test div.</div>
<div id="test4">I am a test div.</div>
<div id="test4b">I am a test div.</div>
<script>
async_test(function (test) {
const iframe = document.createElement("iframe");
iframe.src = "resources/css-module-basic-iframe.html";
iframe.onload = test.step_func_done(function () {
assert_equals(getComputedStyle(iframe.contentDocument.querySelector('#test'))
.backgroundColor, "rgb(255, 0, 0)",
"CSS module import should succeed");
});
document.body.appendChild(iframe);
window.errorCount = 0;
window.onerror = (errorMsg, url, lineNumber, column, errorObj) => {
window.errorCount++;
};
</script>
<script type="module" onerror="unreachable()">
import sheet from "./resources/basic.css" assert { type: "css" };
test(() => {
document.adoptedStyleSheets = [...document.adoptedStyleSheets, sheet];
assert_equals(getComputedStyle(document.querySelector('#test'))
.backgroundColor, "rgb(255, 0, 0)", "CSS module import should succeed");
}, "A CSS Module should load");

async_test(function (test) {
</script>
<script type="module" onerror="unreachable()">
import sheet from "./resources/basic-large.css" assert { type: "css" };
test(() => {
// This tests potential streaming compilation of modules in
// Chromium that is triggered only for large (32>KiB) files in older
// versions.
const iframe = document.createElement("iframe");
iframe.src = "resources/css-module-basic-large-iframe.html";
iframe.onload = test.step_func_done(function () {
assert_equals(getComputedStyle(iframe.contentDocument.querySelector('#test'))
document.adoptedStyleSheets = [...document.adoptedStyleSheets, sheet];
assert_equals(getComputedStyle(document.querySelector('#test2'))
.backgroundColor, "rgb(255, 0, 0)",
"CSS module import should succeed");
});
document.body.appendChild(iframe);
}, "A large CSS Module should load");

async_test(function (test) {
const iframe = document.createElement("iframe");
iframe.src = "resources/css-module-at-import-iframe.html";
iframe.onload = test.step_func_done(function () {
assert_equals(iframe.contentDocument.load_error, undefined);
assert_equals(iframe.contentDocument.adoptedStyleSheets[0].cssRules.length, 1, "Parser should skip @import rule");
assert_not_equals(getComputedStyle(iframe.contentDocument.querySelector('#test'))
.backgroundColor, "rgb(255, 0, 0)",
"CSS module @import should not succeed");
assert_equals(getComputedStyle(iframe.contentDocument.querySelector('#test2'))
.backgroundColor, "rgb(0, 255, 0)",
"Rule after @import should still be applied");
});
document.body.appendChild(iframe);
</script>
<script type="module" onerror="unreachable()">
import sheet from "./resources/bad-import.css" assert { type: "css" };
test(() => {
document.adoptedStyleSheets = [...document.adoptedStyleSheets, sheet];
assert_equals(window.errorCount, 0);
assert_equals(sheet.cssRules.length, 1, "Parser should skip @import rule");
assert_equals(getComputedStyle(document.querySelector('#test3b'))
.backgroundColor, "rgba(0, 0, 0, 0)",
"CSS module @import should not succeed");
assert_equals(getComputedStyle(document.querySelector('#test3'))
.backgroundColor, "rgb(0, 255, 0)",
"Rule after @import should still be applied");
}, "An @import CSS Module should not load, but should not throw an exception");

async_test(function (test) {
const iframe = document.createElement("iframe");
iframe.src = "resources/malformed-iframe.html";
iframe.onload = test.step_func_done(function () {
assert_equals(iframe.contentDocument.load_error, undefined);
assert_equals(iframe.contentDocument.adoptedStyleSheets[0].cssRules.length, 1, "Import of malformed CSS should succeed and rules after the parse error should still be parsed");
assert_not_equals(getComputedStyle(iframe.contentDocument.querySelector('#test'))
.backgroundColor, "rgb(255, 0, 0)",
"Malformed CSS rule should not be applied");
assert_equals(getComputedStyle(iframe.contentDocument.querySelector('#test2'))
.backgroundColor, "rgb(0, 255, 0)",
"Parsing should recover and rules after malformed rules should be applied");
});
document.body.appendChild(iframe);
</script>
<script type="module" onerror="unreachable()">
import sheet from "./resources/malformed.css" assert { type: "css" };
test(() => {
document.adoptedStyleSheets = [...document.adoptedStyleSheets, sheet];
assert_equals(window.errorCount, 0);
assert_equals(sheet.cssRules.length, 1, "Import of malformed CSS should succeed and rules after the parse error should still be parsed");
assert_equals(getComputedStyle(document.querySelector('#test4'))
.backgroundColor, "rgba(0, 0, 0, 0)",
"Malformed CSS rule should not be applied");
assert_equals(getComputedStyle(document.querySelector('#test4b'))
.backgroundColor, "rgb(0, 255, 0)",
"Parsing should recover and rules after malformed rules should be applied");
}, "A parse error should not prevent subsequent rules from being included in a CSS module");

async_test(function (test) {
</script>
<script type="module">
promise_test(function (test) {
const iframe = document.createElement("iframe");
iframe.src = "resources/css-module-without-assertion-iframe.html";
iframe.onload = test.step_func_done(function () {
return new Promise(resolve => {
iframe.onload = resolve;
document.body.appendChild(iframe);
}).then(event => {
assert_equals(iframe.contentDocument.window_onerror, undefined);
assert_equals(iframe.contentDocument.script_onerror.type, "error");
assert_not_equals(getComputedStyle(iframe.contentDocument.querySelector('#test'))
.backgroundColor, "rgb(255, 0, 0)",
assert_equals(getComputedStyle(iframe.contentDocument.querySelector('#test'))
.backgroundColor, "rgba(0, 0, 0, 0)",
"CSS module without type assertion should result in a fetch error");
});
document.body.appendChild(iframe);
}, "CSS module without type assertion should result in a fetch error");
</script>
</body>
3 changes: 3 additions & 0 deletions html/semantics/scripting-1/the-script-element/css-module/resources/atImported.css
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#test3b {
background-color: #FF0000;
}
4 changes: 2 additions & 2 deletions html/semantics/scripting-1/the-script-element/css-module/resources/bad-import.css
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import "basic.css";
#test2 {
@import "atImported.css";
#test3 {
background-color:#00FF00;
}
2 changes: 1 addition & 1 deletion html/semantics/scripting-1/the-script-element/css-module/resources/basic-large.css
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 0 additions & 21 deletions ...tics/scripting-1/the-script-element/css-module/resources/css-module-at-import-iframe.html
View file

This file was deleted.

18 changes: 0 additions & 18 deletions ...emantics/scripting-1/the-script-element/css-module/resources/css-module-basic-iframe.html
View file

This file was deleted.

18 changes: 0 additions & 18 deletions ...cs/scripting-1/the-script-element/css-module/resources/css-module-basic-large-iframe.html
View file

This file was deleted.

14 changes: 0 additions & 14 deletions html/semantics/scripting-1/the-script-element/css-module/resources/malformed-iframe.html
View file

This file was deleted.

4 changes: 2 additions & 2 deletions html/semantics/scripting-1/the-script-element/css-module/resources/malformed.css
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#test } {
#test4 } {
background-color: #FF0000;
}

#test2 {
#test4b {
background-color: #00FF00;
}

0 comments on commit 07a56dc

Please sign in to comment.