Remove unneeded forced SearchModel refresh when search is complete

src/vs/workbench/parts/search/common/searchModel.ts

@@ -791,9 +791,6 @@ export class SearchModel extends Disposable {
 	private onSearchCompleted(completed: ISearchComplete, duration: number): ISearchComplete {
 		this.currentRequest = null;
 
-		if (completed) {
-			this._searchResult.add(completed.results, false);
-		}
 		const options: IPatternInfo = objects.assign({}, this._searchQuery.contentPattern);
 		delete options.pattern;
 		/* __GDPR__

src/vs/workbench/parts/search/test/common/searchModel.test.ts

@@ -77,12 +77,21 @@ suite('SearchModel', () => {
 		});
 	});
 
-	test('Search Model: Search adds to results', function () {
+	function ppromiseWithProgress(results: IFileMatch[]): () => PPromise<any, IFileMatch> {
+		return () => new PPromise((resolve, reject, progress) => {
+			process.nextTick(() => {
+				results.forEach(progress);
+				resolve(null);
+			});
+		});
+	}
+
+	test('Search Model: Search adds to results', async () => {
 		let results = [aRawMatch('file://c:/1', aLineMatch('preview 1', 1, [[1, 3], [4, 7]])), aRawMatch('file://c:/2', aLineMatch('preview 2'))];
-		instantiationService.stub(ISearchService, 'search', PPromise.as({ results: results }));
+		instantiationService.stub(ISearchService, 'search', ppromiseWithProgress(results));
 
 		let testObject: SearchModel = instantiationService.createInstance(SearchModel);
-		testObject.search({ contentPattern: { pattern: 'somestring' }, type: 1, folderQueries });
+		await testObject.search({ contentPattern: { pattern: 'somestring' }, type: 1, folderQueries });
 
 		let actual = testObject.searchResult.matches();
 
@@ -102,59 +111,27 @@ suite('SearchModel', () => {
 		assert.ok(new Range(2, 1, 2, 2).equalsRange(actuaMatches[0].range()));
 	});
 
-	test('Search Model: Search adds to results during progress', function () {
-		let results = [aRawMatch('file://c:/1', aLineMatch('preview 1', 1, [[1, 3], [4, 7]])), aRawMatch('file://c:/2', aLineMatch('preview 2'))];
-		let promise = new DeferredPPromise<ISearchComplete, ISearchProgressItem>();
-		instantiationService.stub(ISearchService, 'search', promise);
-
-		let testObject = instantiationService.createInstance(SearchModel);
-		let result = testObject.search({ contentPattern: { pattern: 'somestring' }, type: 1, folderQueries });
-
-		promise.progress(results[0]);
-		promise.progress(results[1]);
-		promise.complete({ results: [], stats: testSearchStats });
-
-		return result.then(() => {
-			let actual = testObject.searchResult.matches();
-
-			assert.equal(2, actual.length);
-			assert.equal('file://c:/1', actual[0].resource().toString());
-
-			let actuaMatches = actual[0].matches();
-			assert.equal(2, actuaMatches.length);
-			assert.equal('preview 1', actuaMatches[0].text());
-			assert.ok(new Range(2, 2, 2, 5).equalsRange(actuaMatches[0].range()));
-			assert.equal('preview 1', actuaMatches[1].text());
-			assert.ok(new Range(2, 5, 2, 12).equalsRange(actuaMatches[1].range()));
-
-			actuaMatches = actual[1].matches();
-			assert.equal(1, actuaMatches.length);
-			assert.equal('preview 2', actuaMatches[0].text());
-			assert.ok(new Range(2, 1, 2, 2).equalsRange(actuaMatches[0].range()));
-		});
-	});
-
-	test('Search Model: Search reports telemetry on search completed', function () {
+	test('Search Model: Search reports telemetry on search completed', async () => {
 		let target = instantiationService.spy(ITelemetryService, 'publicLog');
 		let results = [aRawMatch('file://c:/1', aLineMatch('preview 1', 1, [[1, 3], [4, 7]])), aRawMatch('file://c:/2', aLineMatch('preview 2'))];
-		instantiationService.stub(ISearchService, 'search', PPromise.as({ results: results }));
+		instantiationService.stub(ISearchService, 'search', ppromiseWithProgress(results));
 
 		let testObject = instantiationService.createInstance(SearchModel);
-		testObject.search({ contentPattern: { pattern: 'somestring' }, type: 1, folderQueries });
+		await testObject.search({ contentPattern: { pattern: 'somestring' }, type: 1, folderQueries });
 
-		assert.ok(target.calledOnce);
+		assert.ok(target.calledThrice);
 		const data = target.args[0];
 		data[1].duration = -1;
-		assert.deepEqual(['searchResultsShown', { count: 3, fileCount: 2, options: {}, duration: -1, useRipgrep: undefined }], data);
+		assert.deepEqual(['searchResultsFirstRender', { duration: -1 }], data);
 	});
 
-	test('Search Model: Search reports timed telemetry on search when progress is not called', function () {
+	test('Search Model: Search reports timed telemetry on search when progress is not called', () => {
 		let target2 = sinon.spy();
 		stub(nullEvent, 'stop', target2);
 		let target1 = sinon.stub().returns(nullEvent);
 		instantiationService.stub(ITelemetryService, 'publicLog', target1);
 
-		instantiationService.stub(ISearchService, 'search', PPromise.as({ results: [] }));
+		instantiationService.stub(ISearchService, 'search', ppromiseWithProgress([]));
 
 		let testObject = instantiationService.createInstance(SearchModel);
 		const result = testObject.search({ contentPattern: { pattern: 'somestring' }, type: 1, folderQueries });
@@ -168,7 +145,7 @@ suite('SearchModel', () => {
 		});
 	});
 
-	test('Search Model: Search reports timed telemetry on search when progress is called', function () {
+	test('Search Model: Search reports timed telemetry on search when progress is called', () => {
 		let target2 = sinon.spy();
 		stub(nullEvent, 'stop', target2);
 		let target1 = sinon.stub().returns(nullEvent);
@@ -192,7 +169,7 @@ suite('SearchModel', () => {
 		});
 	});
 
-	test('Search Model: Search reports timed telemetry on search when error is called', function () {
+	test('Search Model: Search reports timed telemetry on search when error is called', () => {
 		let target2 = sinon.spy();
 		stub(nullEvent, 'stop', target2);
 		let target1 = sinon.stub().returns(nullEvent);
@@ -215,7 +192,7 @@ suite('SearchModel', () => {
 		});
 	});
 
-	test('Search Model: Search reports timed telemetry on search when error is cancelled error', function () {
+	test('Search Model: Search reports timed telemetry on search when error is cancelled error', () => {
 		let target2 = sinon.spy();
 		stub(nullEvent, 'stop', target2);
 		let target1 = sinon.stub().returns(nullEvent);
@@ -238,11 +215,11 @@ suite('SearchModel', () => {
 		});
 	});
 
-	test('Search Model: Search results are cleared during search', function () {
+	test('Search Model: Search results are cleared during search', async () => {
 		let results = [aRawMatch('file://c:/1', aLineMatch('preview 1', 1, [[1, 3], [4, 7]])), aRawMatch('file://c:/2', aLineMatch('preview 2'))];
-		instantiationService.stub(ISearchService, 'search', PPromise.as({ results: results }));
+		instantiationService.stub(ISearchService, 'search', ppromiseWithProgress(results));
 		let testObject: SearchModel = instantiationService.createInstance(SearchModel);
-		testObject.search({ contentPattern: { pattern: 'somestring' }, type: 1, folderQueries });
+		await testObject.search({ contentPattern: { pattern: 'somestring' }, type: 1, folderQueries });
 		assert.ok(!testObject.searchResult.isEmpty());
 
 		instantiationService.stub(ISearchService, 'search', new DeferredPPromise<ISearchComplete, ISearchProgressItem>());
@@ -251,7 +228,7 @@ suite('SearchModel', () => {
 		assert.ok(testObject.searchResult.isEmpty());
 	});
 
-	test('Search Model: Previous search is cancelled when new search is called', function () {
+	test('Search Model: Previous search is cancelled when new search is called', async () => {
 		let target = sinon.spy();
 		instantiationService.stub(ISearchService, 'search', new DeferredPPromise((c, e, p) => { }, target));
 		let testObject: SearchModel = instantiationService.createInstance(SearchModel);
@@ -263,29 +240,29 @@ suite('SearchModel', () => {
 		assert.ok(target.calledOnce);
 	});
 
-	test('getReplaceString returns proper replace string for regExpressions', function () {
+	test('getReplaceString returns proper replace string for regExpressions', async () => {
 		let results = [aRawMatch('file://c:/1', aLineMatch('preview 1', 1, [[1, 3], [4, 7]]))];
-		instantiationService.stub(ISearchService, 'search', PPromise.as({ results: results }));
+		instantiationService.stub(ISearchService, 'search', ppromiseWithProgress(results));
 
 		let testObject: SearchModel = instantiationService.createInstance(SearchModel);
-		testObject.search({ contentPattern: { pattern: 're' }, type: 1, folderQueries });
+		await testObject.search({ contentPattern: { pattern: 're' }, type: 1, folderQueries });
 		testObject.replaceString = 'hello';
 		let match = testObject.searchResult.matches()[0].matches()[0];
 		assert.equal('hello', match.replaceString);
 
-		testObject.search({ contentPattern: { pattern: 're', isRegExp: true }, type: 1, folderQueries });
+		await testObject.search({ contentPattern: { pattern: 're', isRegExp: true }, type: 1, folderQueries });
 		match = testObject.searchResult.matches()[0].matches()[0];
 		assert.equal('hello', match.replaceString);
 
-		testObject.search({ contentPattern: { pattern: 're(?:vi)', isRegExp: true }, type: 1, folderQueries });
+		await testObject.search({ contentPattern: { pattern: 're(?:vi)', isRegExp: true }, type: 1, folderQueries });
 		match = testObject.searchResult.matches()[0].matches()[0];
 		assert.equal('hello', match.replaceString);
 
-		testObject.search({ contentPattern: { pattern: 'r(e)(?:vi)', isRegExp: true }, type: 1, folderQueries });
+		await testObject.search({ contentPattern: { pattern: 'r(e)(?:vi)', isRegExp: true }, type: 1, folderQueries });
 		match = testObject.searchResult.matches()[0].matches()[0];
 		assert.equal('hello', match.replaceString);
 
-		testObject.search({ contentPattern: { pattern: 'r(e)(?:vi)', isRegExp: true }, type: 1, folderQueries });
+		await testObject.search({ contentPattern: { pattern: 'r(e)(?:vi)', isRegExp: true }, type: 1, folderQueries });
 		testObject.replaceString = 'hello$1';
 		match = testObject.searchResult.matches()[0].matches()[0];
 		assert.equal('helloe', match.replaceString);