[SwiftTestTool] Add support for parallel test execution. #583
Conversation
ankitspd
force-pushed
the
parallel-tests
branch
2 times, most recently
from
b5b5038
to
c035ecd
Compare
| private let progressBar: ProgressBarProtocol | ||
|
|
||
| /// A lock to update progress bar status. | ||
| private var progressBarLock = Basic.Lock() |
There was a problem hiding this comment.
Instead of using a lock, I would expect a single status reporting thread which consumes items from the status queue (serially).
There was a problem hiding this comment.
(in this case, run() would probably be the consumer rather than a separate thread)
There was a problem hiding this comment.
so errors will printed during the progress?
There was a problem hiding this comment.
or have a separate queue for status?
There was a problem hiding this comment.
Yeah, they are both part of status reporting in general...
|
Looks like a reasonable start to me, I added a couple minor comments inline and we should try and get at least a minimal functional test (OS X only, for now, due to testing infrastructure) in. |
ankitspd
force-pushed
the
parallel-tests
branch
4 times, most recently
from
8729577
to
be37bcc
Compare
|
@ddunbar Updated |
| pendingTests.enqueue(testCase.name + "/" + test) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
It feels weird to initialize the pendingTests queue in the initializer but consume it inside run(). Any reason not to just to it in run?
| private let pendingTests = SynchronizedQueue<String?>() | ||
|
|
||
| /// The queue containing output of failed tests. | ||
| private let failedTests = SynchronizedQueue<String?>() |
There was a problem hiding this comment.
Why do we have a queue for failed and a queue for finished? Also, wouldn't it be nice to have a structured object inside the queue rather than just String?
ankitspd
force-pushed
the
parallel-tests
branch
from
be37bcc
to
0a9df09
Compare
|
@ddunbar Updated |
| /// Updates the progress bar status. | ||
| private func updateProgress(for test: UnitTest) { | ||
| numCurrentTest += 1 | ||
| progressBar.update(percent: 100*numCurrentTest/numTests, text: "test :: \(test.specifier)") |
There was a problem hiding this comment.
Oh, btw we should just drop the test :: it doesn't really make sense for us.
|
LGTM |
Adds a new `--parallel` mode to SwiftTestTool which spawns threads to execute the tests in parallel. The number of threads spawned is determined by the active processor count. The tests are executed in parallel by first determining test specifiers and then executing one specifier per thread.
ankitspd
force-pushed
the
parallel-tests
branch
from
0a9df09
to
1ae9e60
Compare
No description provided.