#6679 tests and fixes

src/vs/workbench/parts/markers/common/markersModel.ts

@@ -4,20 +4,28 @@
  *--------------------------------------------------------------------------------------------*/
 'use strict';
 
+import * as strings from 'vs/base/common/strings';
+import * as paths from 'vs/base/common/paths';
 import * as Map from 'vs/base/common/map';
 import Severity from 'vs/base/common/severity';
 import URI from 'vs/base/common/uri';
+import { Range } from 'vs/editor/common/core/range';
 import { IMarker, MarkerStatistics } from 'vs/platform/markers/common/markers';
 import {IFilter, or, matchesContiguousSubString, matchesPrefix} from 'vs/base/common/filters';
 import Messages from 'vs/workbench/parts/markers/common/messages';
 
 export class Resource {
-	constructor(public uri: URI, public markers: Marker[], public statistics: MarkerStatistics){};
+	public name: string;
+	public path: string;
+	constructor(public uri: URI, public markers: Marker[], public statistics: MarkerStatistics){
+		this.path= uri.fsPath;
+		this.name= paths.basename(uri.fsPath);
+	}
 }
 
 export class Marker {
 	static _filter: IFilter = or(matchesPrefix, matchesContiguousSubString);
-	constructor(public id:string, public marker: IMarker){};
+	constructor(public id:string, public marker: IMarker){}
 }
 
 export class MarkersModel {
@@ -34,10 +42,13 @@ export class MarkersModel {
 		var resources= <Resource[]>this.markersByResource.entries().map(this.toResource.bind(this));
 		resources= resources.filter((resource) => {return !!resource;});
 		resources.sort((a: Resource, b: Resource) => {
-			if (a.statistics.errors > 0 && b.statistics.errors > 0) {
-				return a.uri.toString().localeCompare(b.uri.toString());
+			if (a.statistics.errors === 0 && b.statistics.errors > 0) {
+				return 1;
+			}
+			if (b.statistics.errors === 0 && a.statistics.errors > 0) {
+				return -1;
 			}
-			return a.statistics.errors > 0 ? -1 : 1;
+			return strings.localeCompare(a.path, b.path) || strings.localeCompare(a.name, b.name);
 		});
 		return resources;
 	}
@@ -82,33 +93,17 @@ export class MarkersModel {
 	}
 
 	private compareMarkers(a: Marker, b:Marker): number {
-		let result= this.compare(a.marker.startLineNumber, b.marker.startLineNumber);
-		if (result !== 0) {
-			return result;
-		}
-
-		result= this.compare(a.marker.startColumn, b.marker.startColumn);
-		if (result !== 0) {
-			return result;
-		}
-
-		result= this.compare(a.marker.endLineNumber, b.marker.endLineNumber);
-		if (result !== 0) {
-			return result;
-		}
-
-		result= this.compare(a.marker.endColumn, b.marker.endColumn);
-		if (result !== 0) {
-			return result;
-		}
-
-		return a.marker.message.localeCompare(b.marker.message);
-	}
-
-	private compare(a: number, b: number): number {
-		return a < b ? -1
-					: a > b ? 1
-					: 0;
+		return Range.compareRangesUsingStarts({
+			startLineNumber: a.marker.startLineNumber,
+			startColumn: a.marker.startColumn,
+			endLineNumber: a.marker.endLineNumber,
+			endColumn: a.marker.endColumn
+		}, {
+			startLineNumber: b.marker.startLineNumber,
+			startColumn: b.marker.startColumn,
+			endLineNumber: b.marker.endLineNumber,
+			endColumn: b.marker.endColumn
+		});
 	}
 
 	private filterMarker(marker: IMarker):boolean {

src/vs/workbench/parts/markers/test/common/markersModel.test.ts

+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+'use strict';
+
+import assert = require('assert');
+import URI from 'vs/base/common/uri';
+import Severity from 'vs/base/common/severity';
+import { IMarker, MarkerStatistics } from 'vs/platform/markers/common/markers';
+import { MarkersModel, Marker, Resource } from 'vs/workbench/parts/markers/common/markersModel';
+
+suite('MarkersModel Test', () => {
+
+	test('getFilteredResource return markers grouped by resource', function() {
+		let marker1= aMarker('res1');
+		let marker2= aMarker('res2');
+		let marker3= aMarker('res1');
+		let marker4= aMarker('res3');
+		let marker5= aMarker('res4');
+		let marker6= aMarker('res2');
+		let testObject= new MarkersModel([marker1, marker2, marker3, marker4, marker5, marker6]);
+
+		let actuals= testObject.getFilteredResources();
+
+		assert.equal(4, actuals.length);
+
+		assert.ok(compareResource(actuals[0], 'res1'));
+		assert.equal(2, actuals[0].markers.length);
+		assert.ok(hasMarker(actuals[0].markers, marker1));
+		assert.ok(hasMarker(actuals[0].markers, marker3));
+
+		assert.ok(compareResource(actuals[1], 'res2'));
+		assert.equal(2, actuals[1].markers.length);
+		assert.ok(hasMarker(actuals[1].markers, marker2));
+		assert.ok(hasMarker(actuals[1].markers, marker6));
+
+		assert.ok(compareResource(actuals[2], 'res3'));
+		assert.equal(1, actuals[2].markers.length);
+		assert.ok(hasMarker(actuals[2].markers, marker4));
+
+		assert.ok(compareResource(actuals[3], 'res4'));
+		assert.equal(1, actuals[3].markers.length);
+		assert.ok(hasMarker(actuals[3].markers, marker5));
+	});
+
+	test('getFilteredResource return markers grouped by resource sorted by file path', function() {
+		let marker1= aMarker('a/res1');
+		let marker2= aMarker('a/res2');
+		let marker3= aMarker('res4');
+		let marker4= aMarker('b/res3');
+		let marker5= aMarker('res4');
+		let marker6= aMarker('c/res2');
+		let testObject= new MarkersModel([marker1, marker2, marker3, marker4, marker5, marker6]);
+
+		let actuals= testObject.getFilteredResources();
+
+		assert.equal(5, actuals.length);
+		assert.ok(compareResource(actuals[0], 'a/res1'));
+		assert.ok(compareResource(actuals[1], 'a/res2'));
+		assert.ok(compareResource(actuals[2], 'b/res3'));
+		assert.ok(compareResource(actuals[3], 'c/res2'));
+		assert.ok(compareResource(actuals[4], 'res4'));
+	});
+
+	test('getFilteredResource return resources with no errors at the end', function() {
+		let marker1= aMarker('a/res1', Severity.Warning);
+		let marker2= aMarker('a/res2');
+		let marker3= aMarker('res4');
+		let marker4= aMarker('b/res3');
+		let marker5= aMarker('res4');
+		let marker6= aMarker('c/res2', Severity.Info);
+		let testObject= new MarkersModel([marker1, marker2, marker3, marker4, marker5, marker6]);
+
+		let actuals= testObject.getFilteredResources();
+
+		assert.equal(5, actuals.length);
+		assert.ok(compareResource(actuals[0], 'a/res2'));
+		assert.ok(compareResource(actuals[1], 'b/res3'));
+		assert.ok(compareResource(actuals[2], 'res4'));
+		assert.ok(compareResource(actuals[3], 'a/res1'));
+		assert.ok(compareResource(actuals[4], 'c/res2'));
+	});
+
+	test('getFilteredResource return markers sorted by line and column', function() {
+		let marker1= aMarkerWithRange(8, 1, 9, 3);
+		let marker2= aMarkerWithRange(3);
+		let marker3= aMarkerWithRange(5);
+		let marker4= aMarkerWithRange(8, 1, 8, 4, 'ab');
+		let marker5= aMarkerWithRange(10);
+		let marker6= aMarkerWithRange(8, 1, 8, 4, 'ba');
+		let marker7= aMarkerWithRange(4);
+		let marker8= aMarkerWithRange(8, 2, 8, 4);
+		let testObject= new MarkersModel([marker1, marker2, marker3, marker4, marker5, marker6, marker7, marker8]);
+
+		let actuals= testObject.getFilteredResources();
+
+		assert.equal(8, actuals[0].markers.length);
+		assert.equal(actuals[0].markers[0].marker, marker2);
+		assert.equal(actuals[0].markers[1].marker, marker7);
+		assert.equal(actuals[0].markers[2].marker, marker3);
+
+		assert.equal(actuals[0].markers[3].marker, marker4);
+		assert.equal(actuals[0].markers[4].marker, marker6);
+		assert.equal(actuals[0].markers[5].marker, marker1);
+		assert.equal(actuals[0].markers[6].marker, marker8);
+
+		assert.equal(actuals[0].markers[7].marker, marker5);
+	});
+
+	function hasResource(resources:Resource[], resource:string):boolean {
+		return resources.filter((r):boolean => {
+			return r.uri.toString() === URI.file(resource).toString();
+		}).length === 1;
+	}
+
+	function hasMarker(markers:Marker[], marker:IMarker):boolean {
+		return markers.filter((m):boolean => {
+			return m.marker === marker;
+		}).length === 1;
+	}
+
+	function compareResource(a:Resource, b:string):boolean {
+		return a.uri.toString() === URI.file(b).toString();
+	}
+
+	function aMarkerWithRange(startLineNumber:number=10,
+					startColumn:number=5,
+					endLineNumber:number= startLineNumber + 1,
+					endColumn:number=startColumn + 5,
+					message: string= 'some message'
+					):IMarker {
+		return aMarker('some resource', Severity.Error, startLineNumber, startColumn, endLineNumber, endColumn, message);
+	}
+
+	function aMarker(resource:string='some resource',
+					severity:Severity=Severity.Error,
+					startLineNumber:number=10,
+					startColumn:number=5,
+					endLineNumber:number= startLineNumber + 1,
+					endColumn:number=startColumn + 5,
+					message:string='some message'
+					):IMarker {
+		return {
+			owner: 'someOwner',
+			resource: URI.file(resource),
+			severity: severity,
+			message: message,
+			startLineNumber: startLineNumber,
+			startColumn: startColumn,
+			endLineNumber: endLineNumber,
+			endColumn: endColumn
+		}
+	}
+});
\ No newline at end of file