New issue
Advanced search Search tips

Issue 711355 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

For testharness.js test output matching, ignore line order.

Project Member Reported by qyears...@chromium.org, Apr 13 2017

Issue description

For testharness.js tests with -expected.txt files, the output is considered "not matching" even if the same subtests PASS and FAIL in exactly the same way but in a different order.

But in general in wpt, order of subtest execution is not an important part of the test.

We could decrease flakiness by sorting lines before comparing actual and expected for testharness tests. We currently have some flaky expectations in TestExpectations that are due to non-deterministic line ordering (e.g. https://chromium.googlesource.com/chromium/src/+/b1267c495884cb9950558f2014633fcf822c7c59/third_party/WebKit/LayoutTests/TestExpectations#1759, bug 705125):

For example, if mytest-expected.txt has:

This is a testharness.js-based test.
PASS "xyz"
FAIL "expected 5, got 4"
PASS "abc"
Harness: the test ran to completion.

And the actual is:

This is a testharness.js-based test.
FAIL "expected 5, got 4"
PASS "xyz"
PASS "abc"
Harness: the test ran to completion.

This could be considered a match.

Does this sound like a good idea? Would it make sense to apply this to all testharness.js tests everywhere including those not in wpt?
 
Without some sort of structure to the assertions, it seems like allowing them to occur in any order could make failures difficult to debug. It's hard to say whether it would actually cause correctness problems as well. 

Comment 2 by jsb...@chromium.org, Apr 14 2017

We did work to make promise_test()s execute in deterministic order.

Perhaps we should push for the same thing for async_test(), although the way that API is structured I think the best we could do is make the output occur in same order that the initial async_test() call was made.

Status: WontFix (was: Unconfirmed)
Right, so it sounds like we probably don't want to do this now.

Re #1: In general, testharness.js subtests are supposed to all have names, and so I think failures might not be made more difficult to debug.

The problem I could see with making this change would be that it would complicate making and showing the diffs for mismatches, and it would be different from how things are done for other test types, thus potentially confusing.


Sign in to add a comment