feat(watch): batch watch runs #551
Conversation
| @@ -573,10 +590,10 @@ export class Extension implements RunHooks { | |||
| // Run either locations or test ids to always be compatible with the test server (it can run either or). | |||
| if (files.length) { | |||
| const fileItems = files.map(f => this._testTree.testItemForFile(f)).filter(Boolean) as vscodeTypes.TestItem[]; | |||
| await this._queueTestRun(fileItems, 'watch'); | |||
| await this._queueWatchRun(fileItems, 'files'); | |||
There was a problem hiding this comment.
Could there be both files and testItems present? If so, the new logic could split them far apart by queueing two batches, and then piling up into the first batch for a while. Perhaps we should queue a combined watch run, and then split between files and items only when running the batch?
There was a problem hiding this comment.
Hmm, didn't think about that. There can be both files and testItems present, yes, if you're watching some files and some items.
The new logic will split them apart, that can get ugly. Tests should be executing roughly in the order of the files being saved, anything else would feel broken.
I don't see how splitting files and items up within the batch would help. Executing more than one test run at a time would be pretty jumpy in the UI I assume.
What's blocking us from sending both files and testItems within the same test run? My understanding is that today, we filter them with AND, but maybe we could allow switching that to OR, just for the test server?
There was a problem hiding this comment.
What's blocking us from sending both files and testItems within the same test run?
Or we do it the other way around and turn files into testID within the extension, just like we do in the UI and in watch mode. Gonna look into that.
There was a problem hiding this comment.
we discussed this in a VC. decided to not ship #553 and keep this change as is. Dima's proposed change would mean that there's one command instead of two, but it wouldn't change anything about the batching
Fixes microsoft/playwright#33443.
When running a long test, or a test that times out, every file change leads to another "watch" run being enqueued. They all pile up in the command queue, and once blocking test finishes, they are all executed one-by-one. If the watched runs are all about the same file, then we'll see the same test being executed multiple times, once for each ⌘+S. This is wasteful.
This PR fixes this by batching together all watched runs. We can't run watched files and watched IDs in the same test, so there's separate batches.