Bug 76022 – META: make test-webkitpy more useful

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

[Dirk Pranke]

I'm planning on adding new features to test-webkitpy over the next week or two, and cleaning up the code to make it more hackable by people other than cjerdonek :).

Roughly, here's the set of features I have in mind:

1) proper command line help
2) better logging control
3) optional profiling info: per-test time, per-suite time, and actual cProfile data
4) optional code coverage info
5) support for directories and files as command line arguments in addition to test names
6) support for marking tests as fast or slow in addition to integration or unit
7) parallel test execution

This is a tracking bug for this work.

[Adam Barth]

Sounds great.

[Dirk Pranke]

logging control and command line help is in bug 76142.

[Dirk Pranke]

bug 76201 adds support for code coverage.

[Dirk Pranke]

Note: bug 77687 has a bunch of the stuff I'm planning to add, but it needs to be cleaned up and broken into landable chunks.

[Dirk Pranke]

bug 76765 adds support for filenames, directories, and packages as command line args.

[Dirk Pranke]

bug 77807 mostly cleans up the logging of individual test execution; the logging patterns now resemble new-run-webkit-tests (but are slightly improved over it, IMO).

[Dirk Pranke]

bug 78175 tracks parallel test execution.

[Adam Barth]

> 7) parallel test execution

I'm not sure (7) is worth doing.  I'd rather make the tests faster than run them in parallel.  Running the default set of tests takes less than a minute:

Ran 1306 tests in 53.808s

We have 6% of the tests marked as "integration tests", and running test-webkitpy --skip-integrationtests takes less than half the time:

Ran 1228 tests in 23.473s

Rather than add complexity by making the test suite parallel, it seems like breaking down these slow tests into more targeted unit tests has a better return on investment.

[Dirk Pranke]

(In reply to comment #8)
> > 7) parallel test execution
> 
> I'm not sure (7) is worth doing.  I'd rather make the tests faster than run them in parallel.  Running the default set of tests takes less than a minute:
> 
> Ran 1306 tests in 53.808s
> 
> We have 6% of the tests marked as "integration tests", and running test-webkitpy --skip-integrationtests takes less than half the time:
> 
> Ran 1228 tests in 23.473s
> 
> Rather than add complexity by making the test suite parallel, it seems like breaking down these slow tests into more targeted unit tests has a better return on investment.

I don't see this as an either/or thing. Making the tests parallelizable should be pretty easy (I'm most of the way there now), and is forcing me to clean up the messaging code. 

I agree that making the other tests faster would be good also. That said, as I noted in the other bug, there are other integration tests that we're currently not running because they are slow; I would like to enable them as well.

[Adam Barth]

Parallelizing has a cost, which is complexity.  I'm not sure we get enough of a benefit to justify the cost.

[Dirk Pranke]

(In reply to comment #10)
> Parallelizing has a cost, which is complexity.  I'm not sure we get enough of a benefit to justify the cost.

Sure, that is a fair concern. Of course, refactoring the test code to speed it up introduces complexity as well.

My hope is that once this code is cleaned up, adding parallelism will be dirt-simple and it'll be a no-brainer; doing so should only take me a little more time, so I'd prefer to defer further discussion over whether it will be worth it or not until I have something we can all stare at.

As I've said before, the changes I've posted thus far are (IMO) worth it even if we don't end up doing making test-webkitpy parallel, since they're making the code cleaner and easier to understand regardless. I'd like to keep the two issues separate as much as possible.

[Adam Barth]

Sounds like a good plan.

[Dirk Pranke]

Okay, I've got some preliminary data and patch which I will post to bug 78175 ... let's move all further discussion of the pros and cons of parallelization there.

[Dirk Pranke]

whoops, bug 78185, not bug 78175.

[Dirk Pranke]

As an aside, what was 53 seconds yesterday is now 61 seconds ... I wonder what the best way to watch for regressions here would be (or when we should care, given that a minute still isn't that long a time).

[Dirk Pranke]

Created attachment 149666 [details]
roll-up patch based off of r121304 + att. 149665 on bug 89725

This is a roll-up patch that adds parallel execution, better logging, and general code cleanup/restructuring to test-webkitpy. It re-uses the manager_worker_broker logic in webkitpy.layout_tests; the overhead in lines of code for doing so is minimal. However, there's also not a huge win to running in parallel; execution time drops from 20s currently to ~5 seconds with 8 children.

Note that ~45 of the 1400 tests account for about half of the execution time, but o test takes longer than a second to complete (you can generate the list with -t/--timing).

The patch needs better tests, and depends on bug 89725. It also needs to be broken up into more reviewable chunks.

[Dirk Pranke]

Okay, all patches posted, I think.

[Dirk Pranke]

All of the work filed in comment #1 is now complete with the exception of per-suite time and cProfile data for profiling, and support for marking tests as fast or slow. test-webkitpy completes in 20 seconds run serially, and in 4 seconds when run w/ 8 threads.

I think this is good enough to close the bug.