Bug 78750

The problem is that the test is in the Skipped file, and we have code that assumes that all tests that are in Skipped files are expected to FAIL . So, your test is failing, which means that it is running "as expected". I could see how this would be confusing, and it'd probably be better if we marked them as expected to PASS, so that when we do --force the test, it is reported as failing by default.

Shouldn't we assume that all tests in the Skipped file are marked SKIP ?

probably. 'SKIP' isn't (or at least didn't used to be) a valid (right-hand-side) expectation for a test. If that is now legal, it would be better. I was thinking that some expectation like "unknown" would be ideal, since we don't actually know what the expected result is for tests in a skipped file; SKIP would work fine for that.

Ah, I see. Yeah, PASS is probably a good default then.

Created attachment 127307 [details] Patch

Dirk: So what would the new output be for ap, after your change?

Good question :) $ run-webkit-tests --force --platform test --no-new-test-results --no-retry-failures \ failures/expected/skip_text.html failures/unexpected/skip_pass.html failures/expected/skip_text.html -> unexpected text diff mismatch failures/unexpected/skip_pass.html -> unexpected pass 0 tests ran as expected, 2 didn't: Expected to skip, but passed: (1) failures/unexpected/skip_pass.html Regressions: Unexpected text diff mismatch : (1) failures/expected/skip_text.html = TEXT MOCK: user.open_url: file://... $ Note the failing test doesn't say "Expected to skip, but failed with a text diff mismatch"; there is no indication that it was marked as SKIP ... I can probably make something like that happen if there is a desire, but that will be a more invasive change, I suspect.

What's the output with the original proposed fix (where being in Skipped file means "pass")? IIRC, it was better.

(In reply to comment #8) > What's the output with the original proposed fix (where being in Skipped file means "pass")? IIRC, it was better. $ new-run-webkit-tests --force --no-new-test-results --no-retry-failures --platform test failures/unexpected/skip_pass.html failures/expected/skip_text.html failures/expected/skip_text.html -> unexpected text diff mismatch 1 test ran as expected, 1 didn't: Regressions: Unexpected text diff mismatch : (1) failures/expected/skip_text.html = TEXT MOCK: user.open_url: file://... layout_tests $ You get the message about the test that fails, but not the test that passes. I think the first example (and hence the patch that I uploaded) is better, but I don't feel strongly about it; if you do, I can drop the code that prints the unexpected passes.

For my purposes, the simple fix (with output as in comment 9) works perfectly.

Eric, Adam, Tony, Ojan, any of you have any leanings?

Created attachment 127647 [details] argh ... ignore; wrong bug.

*** Bug 78931 has been marked as a duplicate of this bug. ***

I prefer the less verbose output of comment 9. In general, the output when there are failures is still too verbose IMO. Anything we can do to reduce it is better. In this case, the 'Expected to skip, but passed' doesn't really provide any actionable information IMO.

(In reply to comment #14) > I prefer the less verbose output of comment 9. Alrighty then. Two votes for terseness is enough for me. > In general, the output when there are failures is still too verbose IMO. Ojan: so (independent of this change) what would you kill in the output for failures? the summary at the end? the failures as they occur? Feel free to file another bug and/or mail me off-bugzilla.

Created attachment 127668 [details] make Skipped tests expected to PASS instead of SKIP

Comment on attachment 127668 [details] make Skipped tests expected to PASS instead of SKIP OK.

Committed r108143: <http://trac.webkit.org/changeset/108143>