Bug 86800 – [NRWT] REGRESSION(r116021): Assertion when running with --no-http

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Raphael Kubo da Costa (:rakuco)]

When running `./Tools/Scripts/run-webkit-tests --efl --debug --no-http' I hit the following assertion:

Traceback (most recent call last):
  File "/home/rakuco/dev/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 457, in <module>
    sys.exit(main())
  File "/home/rakuco/dev/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 452, in main
    return run(port, options, args)
  File "/home/rakuco/dev/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 125, in run
    unexpected_result_count = manager.run()
  File "/home/rakuco/dev/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 906, in run
    interrupted, keyboard_interrupted, thread_timings, test_timings, individual_test_timings = self._run_tests(self._test_files_list, result_summary, int(self._options.child_processes))
  File "/home/rakuco/dev/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 756, in _run_tests
    self.start_servers_with_lock()
  File "/home/rakuco/dev/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 955, in start_servers_with_lock
    assert(self._options.http)

[Dirk Pranke]

Looks like this is caused by including perf tests in the "locked" shard.

[Raphael Kubo da Costa (:rakuco)]

Added in <http://trac.webkit.org/changeset/116021>.

[Raphael Kubo da Costa (:rakuco)]

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

[Tony Chang]

We should roll out if no one is going to get a chance to investigate this soon.

[Dirk Pranke]

(In reply to comment #4)
> We should roll out if no one is going to get a chance to investigate this soon.

I'll fix this later today.

[Thiago Marcos P. Santos]

Created attachment 143777 [details]
Patch

[Adam Barth]

Comment on attachment 143777 [details]
Patch

Can we write a test that will catch this problem in the future?

[Dirk Pranke]

This patch looks fine, but I think it might actually be better to revert the change that added the perf tests to the locked shards, since that didn't actually do what Tony thought it did, and I'm not sure if it had the intended effect. Tony, WDYT?

[Thiago Marcos P. Santos]

(In reply to comment #8)
> This patch looks fine, but I think it might actually be better to revert the change that added the perf tests to the locked shards, since that didn't actually do what Tony thought it did, and I'm not sure if it had the intended effect. Tony, WDYT?

I was the one who wrote the original patch moving perf to locked shards. For the EFL Debug bot at least, it solved the flakiness issue.

And yes, I can write utests for it.

[Tony Chang]

(In reply to comment #8)
> This patch looks fine, but I think it might actually be better to revert the change that added the perf tests to the locked shards, since that didn't actually do what Tony thought it did, and I'm not sure if it had the intended effect. Tony, WDYT?

Either is fine with me.  It sounds like there were benefits of this patch for tmpsantos, so I don't mind keeping it.  It still ensures that we don't mix http tests with perf tests, right?

[Dirk Pranke]

Oh, sorry, I thought Tony did the original patch.

At any rate, the change looks fine so I'll r+ it and if it's valuable to you feel free to land it.

[Thiago Marcos P. Santos]

(In reply to comment #11)
> Oh, sorry, I thought Tony did the original patch.
> 
> At any rate, the change looks fine so I'll r+ it and if it's valuable to you feel free to land it.

I might need someone with cq+ superpowers then. :)

[WebKit Review Bot]

Comment on attachment 143777 [details]
Patch

Clearing flags on attachment: 143777

Committed r118421: <http://trac.webkit.org/changeset/118421>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.