webkitpy: Fixing tests broken via WontFix change.

webkitpy: Fixing tests broken via WontFix change.

This commit reverts the changes in
a2f07ea5d0d9fd81a7b86089fa2826518f44fdad to the test suite and correctly
makes the tests as WontFix.

The change only previously worked because
failures/expected/exception.html occured after
failures/expected/device_failure.html (which cancelled the test suite
early). This can be seen in the test counts being strange.

It removes the --order=natural argument to make sure tests continue to
work in random order.

BUG=524758
Review-Url: https://codereview.chromium.org/2694703006
Cr-Commit-Position: refs/heads/master@{#450602}
Committed: https://chromium.googlesource.com/chromium/src/+/ff34f059f6711e3473ec0191e20550c77732a084

[mithro, 2017-02-14 08:35:29]

The CQ bit was checked by tansell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-14 08:35:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mithro, 2017-02-14 08:36:35]

Description was changed from

==========
webkitpy: Fixing test cases.

This commit reverts the changes in
a2f07ea5d0d9fd81a7b86089fa2826518f44fdad to the test suite and correctly
makes the tests as WontFix.

The change only previously worked because
failures/expected/exception.html occured after
failures/expected/device_failure.html (which cancelled the test suite
early). This can be seen in the test counts being strange.

It removes the --order=natural argument to make sure tests continue to
work in random order.

BUG=524758
==========

to

==========
webkitpy: Fixing tests broken via WontFix change.

This commit reverts the changes in
a2f07ea5d0d9fd81a7b86089fa2826518f44fdad to the test suite and correctly
makes the tests as WontFix.

The change only previously worked because
failures/expected/exception.html occured after
failures/expected/device_failure.html (which cancelled the test suite
early). This can be seen in the test counts being strange.

It removes the --order=natural argument to make sure tests continue to
work in random order.

BUG=524758
==========

[mithro, 2017-02-14 08:37:03]

tansell@chromium.org changed reviewers:
+ dpranke@google.com, mcgreevy@chromium.org, nainar@chromium.org

[mithro, 2017-02-14 08:47:11]

Hi!

This change reverts a bunch of changes in nainar's "Linter should enforce Tests
marked Slow or WontFix aren't allowed in TestExpectations" CL at
https://chromium.googlesource.com/chromium/src/+/a2f07ea5d0d9f.

It restores WontFix status to failures/expected/keyboard.html,
failures/expected/exception.html and failures/expected/device_failure.html and
thus these tests are now correctly skipped most of the time. This can be seen in
that it fixes a bunch of tests which where not returning URLs after nainar's
change but according to their descriptions should have been.

The only reason more tests where not failing currently is that
failures/expected/device_failure.html normally runs before
failures/expected/exception.html causing failures/expected/exception.html to
never be reached. If you change the order the tests are run in, then tests will
start exploding with an exception like this;
------------------------------------------------------------------
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py",
line 163, in _run_compare_test
      driver_output = self._driver.run_test(self._driver_input(),
self._stop_when_done)
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/test.py",
line 563, in run_test
      raise ValueError('exception from ' + test_name)
  ValueError: exception from failures/expected/exception.html
------------------------------------------------------------------

Now it's 1:00am here, so I'm heading to bed.

Tim 'mithro' Ansell

[commit-bot: I haz the power, 2017-02-14 09:42:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-14 09:42:05]

Dry run: This issue passed the CQ dry run.

[mithro, 2017-02-14 18:30:23]

tansell@chromium.org changed reviewers:
+ jeffcarp@chromium.org, qyearsley@chromium.org

[jeffcarp, 2017-02-14 18:36:32]

lgtm as-is with questions

https://codereview.chromium.org/2694703006/diff/1/third_party/WebKit/Tools/Sc...
File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py
(left):

https://codereview.chromium.org/2694703006/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:176:
extra_args=['--json-test-results', '/tmp/json_test_results.json', '--order',
'natural'],
Sorry I'm still not clear why removing --order=natural is necessary?

https://codereview.chromium.org/2694703006/diff/1/third_party/WebKit/Tools/Sc...
File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py
(right):

https://codereview.chromium.org/2694703006/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:753:
self.assertTrue('Retrying' in err.getvalue())
Is there a reason this is assertTrue instead of assertIn now?

[qyearsley, 2017-02-14 19:06:16]

LGTM - to be clear, this only affects tests, and it fixes the tests to properly
cover behavior involving WontFix expectations, right?

[mithro, 2017-02-14 19:22:44]

On 2017/02/14 19:06:16, qyearsley wrote:
> LGTM - to be clear, this only affects tests, and it fixes the tests to
properly
> cover behavior involving WontFix expectations, right?

Yes, this change only affects the tests + mocking. 

It makes them behave correctly with nainar's requirement that  WontFix
expectations are only in the NeverFixTests file.

https://codereview.chromium.org/2694703006/diff/1/third_party/WebKit/Tools/Sc...
File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py
(left):

https://codereview.chromium.org/2694703006/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:176:
extra_args=['--json-test-results', '/tmp/json_test_results.json', '--order',
'natural'],
On 2017/02/14 18:36:32, jeffcarp wrote:
> Sorry I'm still not clear why removing --order=natural is necessary?

It's not necessary (the test works with '--order=natural'). However, using
'--order=natural' means the tests run in the same order every time and it hides
accidental dependencies on test ordering.

https://codereview.chromium.org/2694703006/diff/1/third_party/WebKit/Tools/Sc...
File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py
(right):

https://codereview.chromium.org/2694703006/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:753:
self.assertTrue('Retrying' in err.getvalue())
On 2017/02/14 18:36:32, jeffcarp wrote:
> Is there a reason this is assertTrue instead of assertIn now?

Nope! Fixing.

[nainar, 2017-02-14 23:25:00]

lgtm

[mithro, 2017-02-15 02:13:22]

The CQ bit was checked by tansell@chromium.org

[mithro, 2017-02-15 02:13:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
qyearsley@chromium.org, jeffcarp@chromium.org
Link to the patchset: https://codereview.chromium.org/2694703006/#ps20001
(title: "Fixing assertIn")

[commit-bot: I haz the power, 2017-02-15 02:14:19]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-15 06:42:29]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487124802591370,
"parent_rev": "14236039cf9e0464397a65aaee8e81f0fa6a6e2a", "commit_rev":
"ff34f059f6711e3473ec0191e20550c77732a084"}

[commit-bot: I haz the power, 2017-02-15 06:43:18]

Description was changed from

==========
webkitpy: Fixing tests broken via WontFix change.

This commit reverts the changes in
a2f07ea5d0d9fd81a7b86089fa2826518f44fdad to the test suite and correctly
makes the tests as WontFix.

The change only previously worked because
failures/expected/exception.html occured after
failures/expected/device_failure.html (which cancelled the test suite
early). This can be seen in the test counts being strange.

It removes the --order=natural argument to make sure tests continue to
work in random order.

BUG=524758
==========

to

==========
webkitpy: Fixing tests broken via WontFix change.

This commit reverts the changes in
a2f07ea5d0d9fd81a7b86089fa2826518f44fdad to the test suite and correctly
makes the tests as WontFix.

The change only previously worked because
failures/expected/exception.html occured after
failures/expected/device_failure.html (which cancelled the test suite
early). This can be seen in the test counts being strange.

It removes the --order=natural argument to make sure tests continue to
work in random order.

BUG=524758

Review-Url: https://codereview.chromium.org/2694703006
Cr-Commit-Position: refs/heads/master@{#450602}
Committed:
https://chromium.googlesource.com/chromium/src/+/ff34f059f6711e3473ec0191e205...
==========

[commit-bot: I haz the power, 2017-02-15 06:43:21]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/ff34f059f6711e3473ec0191e205...