Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Commit 143aac5

Browse files
dangoordangoor
committed
Merge pull request #7650 from adobe/pflynn/lint-tests-fixes
Code Inspection: minor fixes & more unit tests
2 parents e3c174d + 168b174 commit 143aac5

3 files changed

Lines changed: 187 additions & 45 deletions

File tree

‎src/language/CodeInspection.js‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ define(function (require, exports, module) {
383383
aborted = true;
384384
}
385385

386-
if (inspectionResult.result.errors) {
386+
if (inspectionResult.result.errors.length) {
387387
allErrors.push({
388388
providerName: provider.name,
389389
results: inspectionResult.result.errors
@@ -436,18 +436,18 @@ define(function (require, exports, module) {
436436
* Brackets JSLint provider. This is a temporary convenience until UI exists for disabling
437437
* registered providers.
438438
*
439-
* If provider implements both scanFileAsync and scanFile functions for asynchronous and synchronous
440-
* code inspection, respectively, the asynchronous version will take precedence and will be used to
441-
* perform code inspection.
442-
*
443-
* A code inspection provider's scanFileAsync must return a {$.Promise} object which should be
444-
* resolved with ?{errors:!Array, aborted:boolean}}.
445-
*
439+
* Providers implement scanFile() if results are available synchronously, or scanFileAsync() if results
440+
* may require an async wait (if both are implemented, scanFile() is ignored). scanFileAsync() returns
441+
* a {$.Promise} object resolved with the same type of value as scanFile() is expected to return.
442+
* Rejecting the promise is treated as an internal error in the provider.
443+
*
446444
* @param {string} languageId
447-
* @param {{name:string, scanFileAsync:?function(string, string):!{$.Promise}, scanFile:?function(string, string):?{errors:!Array, aborted:boolean}}} provider
445+
* @param {{name:string, scanFileAsync:?function(string, string):!{$.Promise},
446+
* scanFile:?function(string, string):?{errors:!Array, aborted:boolean}}} provider
448447
*
449448
* Each error is: { pos:{line,ch}, endPos:?{line,ch}, message:string, type:?Type }
450449
* If type is unspecified, Type.WARNING is assumed.
450+
* If no errors found, return either null or an object with a zero-length `errors` array.
451451
*/
452452
function register(languageId, provider) {
453453
if (!_providers[languageId]) {

‎src/nls/root/strings.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,6 @@ define({
180180
"FILE_FILTER_INSTRUCTIONS" : "Exclude files and folders matching any of the following strings / substrings or <a href='{0}' title='{0}'>wildcards</a>. Enter each string on a new line.",
181181
"FILE_FILTER_LIST_PREFIX" : "except",
182182
"FILE_FILTER_CLIPPED_SUFFIX" : "and {0} more",
183-
184183
"FILTER_COUNTING_FILES" : "Counting files\u2026",
185184
"FILTER_FILE_COUNT" : "Allows {0} of {1} files {2}",
186185
"FILTER_FILE_COUNT_ALL" : "Allows all {0} files {1}",

‎test/spec/CodeInspection-test.js‎

Lines changed: 178 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ define(function (require, exports, module) {
4242
CodeInspection,
4343
CommandManager,
4444
Commands = require("command/Commands"),
45+
DocumentManager,
4546
EditorManager,
4647
prefs;
4748

@@ -111,6 +112,7 @@ define(function (require, exports, module) {
111112
brackets = testWindow.brackets;
112113
Strings = testWindow.require("strings");
113114
CommandManager = brackets.test.CommandManager;
115+
DocumentManager = brackets.test.DocumentManager;
114116
EditorManager = brackets.test.EditorManager;
115117
prefs = brackets.test.PreferencesManager.getExtensionPrefs("linting");
116118
CodeInspection = brackets.test.CodeInspection;
@@ -137,6 +139,7 @@ define(function (require, exports, module) {
137139
$ = null;
138140
brackets = null;
139141
CommandManager = null;
142+
DocumentManager = null;
140143
EditorManager = null;
141144
SpecRunnerUtils.closeTestWindow();
142145
});
@@ -413,8 +416,29 @@ define(function (require, exports, module) {
413416
describe("Code Inspection UI", function () {
414417
beforeEach(function () {
415418
CodeInspection._unregisterAll();
419+
CodeInspection.toggleEnabled(true);
416420
});
417421

422+
// Utility to create an async provider where the testcase can control when each async result resolves
423+
function makeAsyncLinter() {
424+
return {
425+
name: "Test Async Linter",
426+
scanFileAsync: function (text, fullPath) {
427+
if (!this.futures[fullPath]) {
428+
this.futures[fullPath] = [];
429+
this.filesCalledOn.push(fullPath);
430+
}
431+
432+
var result = new $.Deferred();
433+
this.futures[fullPath].push(result);
434+
return result.promise();
435+
},
436+
futures: {}, // map from full path to array of Deferreds (in call order)
437+
filesCalledOn: [] // in order of first call for each path
438+
};
439+
}
440+
441+
418442
it("should run test linter when a JavaScript document opens and indicate errors in the panel", function () {
419443
var codeInspector = createCodeInspector("javascript linter", failLintResult());
420444
CodeInspection.register("javascript", codeInspector);
@@ -427,45 +451,124 @@ define(function (require, exports, module) {
427451
expect($statusBar.is(":visible")).toBe(true);
428452
});
429453
});
430-
431-
it("should show only warnings for the current file", function () {
454+
455+
it("should ignore async results from previous file", function () {
432456
CodeInspection.toggleEnabled(false);
433457

434-
var firstTime = true,
435-
deferred1 = new $.Deferred(),
436-
deferred2 = new $.Deferred();
437-
438-
var asyncProvider = {
439-
name: "Test Async Linter",
440-
scanFileAsync: function () {
441-
if (firstTime) {
442-
firstTime = false;
443-
return deferred1.promise();
444-
} else {
445-
return deferred2.promise();
446-
}
447-
}
448-
};
449-
458+
var asyncProvider = makeAsyncLinter();
450459
CodeInspection.register("javascript", asyncProvider);
451460

452-
waitsForDone(SpecRunnerUtils.openProjectFiles(["no-errors.js", "errors.js"], "open test files"));
461+
waitsForDone(SpecRunnerUtils.openProjectFiles(["no-errors.js", "errors.js"]), "open test files");
462+
463+
var errorsJS = SpecRunnerUtils.makeAbsolute("errors.js"),
464+
noErrorsJS = SpecRunnerUtils.makeAbsolute("no-errors.js");
453465

454466
runs(function () {
467+
// Start linting the first file
455468
CodeInspection.toggleEnabled(true);
456-
CommandManager.execute(Commands.FILE_CLOSE);
469+
expect(asyncProvider.filesCalledOn).toEqual([errorsJS]);
470+
471+
// Close that file, switching to the 2nd one
472+
waitsForDone(CommandManager.execute(Commands.FILE_CLOSE));
457473
});
458474

459-
// Close the file which was started to lint
460475
runs(function () {
461-
// let the linter finish
462-
deferred1.resolve(failLintResult());
476+
// Verify that we started linting the 2nd file
477+
expect(DocumentManager.getCurrentDocument().file.fullPath).toBe(noErrorsJS);
478+
expect(asyncProvider.filesCalledOn).toEqual([errorsJS, noErrorsJS]);
479+
480+
// Finish old (stale) linting session - verify results not shown
481+
asyncProvider.futures[errorsJS][0].resolve(failLintResult());
463482
expect($("#problems-panel").is(":visible")).toBe(false);
464-
deferred2.resolve(successfulLintResult());
483+
484+
// Finish new (current) linting session
485+
asyncProvider.futures[noErrorsJS][0].resolve(successfulLintResult());
465486
expect($("#problems-panel").is(":visible")).toBe(false);
466487
});
467488
});
489+
490+
it("should ignore async results from previous run in same file - finishing in order", function () {
491+
CodeInspection.toggleEnabled(false);
492+
493+
var asyncProvider = makeAsyncLinter();
494+
CodeInspection.register("javascript", asyncProvider);
495+
496+
waitsForDone(SpecRunnerUtils.openProjectFiles(["no-errors.js"]), "open test files");
497+
498+
var noErrorsJS = SpecRunnerUtils.makeAbsolute("no-errors.js");
499+
500+
runs(function () {
501+
// Start linting the file
502+
CodeInspection.toggleEnabled(true);
503+
expect(asyncProvider.filesCalledOn).toEqual([noErrorsJS]);
504+
505+
// "Modify" the file
506+
$(DocumentManager).triggerHandler("documentSaved", DocumentManager.getCurrentDocument());
507+
expect(asyncProvider.futures[noErrorsJS].length).toBe(2);
508+
509+
// Finish old (stale) linting session - verify results not shown
510+
asyncProvider.futures[noErrorsJS][0].resolve(failLintResult());
511+
expect($("#problems-panel").is(":visible")).toBe(false);
512+
513+
// Finish new (current) linting session - verify results are shown
514+
asyncProvider.futures[noErrorsJS][1].resolve(failLintResult());
515+
expect($("#problems-panel").is(":visible")).toBe(true);
516+
});
517+
});
518+
519+
it("should ignore async results from previous run in same file - finishing reverse order", function () {
520+
CodeInspection.toggleEnabled(false);
521+
522+
var asyncProvider = makeAsyncLinter();
523+
CodeInspection.register("javascript", asyncProvider);
524+
525+
waitsForDone(SpecRunnerUtils.openProjectFiles(["no-errors.js"]), "open test files");
526+
527+
var noErrorsJS = SpecRunnerUtils.makeAbsolute("no-errors.js");
528+
529+
runs(function () {
530+
// Start linting the file
531+
CodeInspection.toggleEnabled(true);
532+
expect(asyncProvider.filesCalledOn).toEqual([noErrorsJS]);
533+
534+
// "Modify" the file
535+
$(DocumentManager).triggerHandler("documentSaved", DocumentManager.getCurrentDocument());
536+
expect(asyncProvider.futures[noErrorsJS].length).toBe(2);
537+
538+
// Finish new (current) linting session - verify results are shown
539+
asyncProvider.futures[noErrorsJS][1].resolve(failLintResult());
540+
expect($("#problems-panel").is(":visible")).toBe(true);
541+
542+
// Finish old (stale) linting session - verify results don't replace current results
543+
asyncProvider.futures[noErrorsJS][0].resolve(successfulLintResult());
544+
expect($("#problems-panel").is(":visible")).toBe(true);
545+
});
546+
});
547+
548+
it("should ignore async results after linting disabled", function () {
549+
CodeInspection.toggleEnabled(false);
550+
551+
var asyncProvider = makeAsyncLinter();
552+
CodeInspection.register("javascript", asyncProvider);
468553

554+
waitsForDone(SpecRunnerUtils.openProjectFiles(["no-errors.js"]), "open test files");
555+
556+
var noErrorsJS = SpecRunnerUtils.makeAbsolute("no-errors.js");
557+
558+
runs(function () {
559+
// Start linting the file
560+
CodeInspection.toggleEnabled(true);
561+
expect(asyncProvider.filesCalledOn).toEqual([noErrorsJS]);
562+
563+
// Disable linting
564+
CodeInspection.toggleEnabled(false);
565+
566+
// Finish old (stale) linting session - verify results not shown
567+
asyncProvider.futures[noErrorsJS][0].resolve(failLintResult());
568+
expect($("#problems-panel").is(":visible")).toBe(false);
569+
});
570+
});
571+
469572
it("should show problems panel after too many errors", function () {
470573
var lintResult = {
471574
errors: [
@@ -517,8 +620,8 @@ define(function (require, exports, module) {
517620
});
518621
});
519622

520-
it("should not show the problems panel when there is no linting error", function () {
521-
var codeInspector = createCodeInspector("javascript linter", successfulLintResult());
623+
it("should not show the problems panel when there is no linting error - empty errors array", function () {
624+
var codeInspector = createCodeInspector("javascript linter", {errors: []});
522625
CodeInspection.register("javascript", codeInspector);
523626

524627
waitsForDone(SpecRunnerUtils.openProjectFiles(["errors.js"]), "open test file", 5000);
@@ -530,19 +633,20 @@ define(function (require, exports, module) {
530633
});
531634
});
532635

533-
it("status icon should toggle Errors panel when errors present", function () {
534-
var codeInspector = createCodeInspector("javascript linter", failLintResult());
636+
it("should not show the problems panel when there is no linting error - null result", function () {
637+
var codeInspector = createCodeInspector("javascript linter", null);
535638
CodeInspection.register("javascript", codeInspector);
536639

537-
waitsForDone(SpecRunnerUtils.openProjectFiles(["errors.js"]), "open test file");
640+
waitsForDone(SpecRunnerUtils.openProjectFiles(["errors.js"]), "open test file", 5000);
538641

539642
runs(function () {
540-
toggleJSLintResults(false);
541-
toggleJSLintResults(true);
643+
expect($("#problems-panel").is(":visible")).toBe(false);
644+
var $statusBar = $("#status-inspection");
645+
expect($statusBar.is(":visible")).toBe(true);
542646
});
543647
});
544648

545-
it("should run two linters and display two expanded collapsible sections in the errors panel", function () {
649+
it("should display two expanded, collapsible sections in the errors panel when two linters have errors", function () {
546650
var codeInspector1 = createCodeInspector("javascript linter 1", failLintResult());
547651
var codeInspector2 = createCodeInspector("javascript linter 2", failLintResult());
548652
CodeInspection.register("javascript", codeInspector1);
@@ -561,17 +665,56 @@ define(function (require, exports, module) {
561665
});
562666
});
563667

564-
it("should run the linter and display no collapsible header section in the errors panel", function () {
565-
var codeInspector1 = createCodeInspector("javascript linter 1", failLintResult());
668+
it("should display no header section when only one linter has errors", function () {
669+
var codeInspector1 = createCodeInspector("javascript linter 1", failLintResult()),
670+
codeInspector2 = createCodeInspector("javascript linter 2", {errors: []}), // 1st way of reporting 0 errors
671+
codeInspector3 = createCodeInspector("javascript linter 3", null); // 2nd way of reporting 0 errors
566672
CodeInspection.register("javascript", codeInspector1);
673+
CodeInspection.register("javascript", codeInspector2);
674+
CodeInspection.register("javascript", codeInspector3);
567675

568676
waitsForDone(SpecRunnerUtils.openProjectFiles(["errors.js"]), "open test file", 5000);
569677

570678
runs(function () {
679+
expect($("#problems-panel").is(":visible")).toBe(true);
571680
expect($(".inspector-section").is(":visible")).toBeFalsy();
572681
});
573682
});
574683

684+
it("should only display header sections for linters with errors", function () {
685+
var codeInspector1 = createCodeInspector("javascript linter 1", failLintResult()),
686+
codeInspector2 = createCodeInspector("javascript linter 2", {errors: []}), // 1st way of reporting 0 errors
687+
codeInspector3 = createCodeInspector("javascript linter 3", null), // 2nd way of reporting 0 errors
688+
codeInspector4 = createCodeInspector("javascript linter 4", failLintResult());
689+
CodeInspection.register("javascript", codeInspector1);
690+
CodeInspection.register("javascript", codeInspector2);
691+
CodeInspection.register("javascript", codeInspector3);
692+
CodeInspection.register("javascript", codeInspector4);
693+
694+
waitsForDone(SpecRunnerUtils.openProjectFiles(["errors.js"]), "open test file", 5000);
695+
696+
runs(function () {
697+
expect($("#problems-panel").is(":visible")).toBe(true);
698+
699+
var $inspectorSections = $(".inspector-section td");
700+
expect($inspectorSections.length).toEqual(2);
701+
expect($inspectorSections[0].innerHTML.indexOf("javascript linter 1 (1)")).not.toBe(-1);
702+
expect($inspectorSections[1].innerHTML.indexOf("javascript linter 4 (1)")).not.toBe(-1);
703+
});
704+
});
705+
706+
it("status icon should toggle Errors panel when errors present", function () {
707+
var codeInspector = createCodeInspector("javascript linter", failLintResult());
708+
CodeInspection.register("javascript", codeInspector);
709+
710+
waitsForDone(SpecRunnerUtils.openProjectFiles(["errors.js"]), "open test file");
711+
712+
runs(function () {
713+
toggleJSLintResults(false);
714+
toggleJSLintResults(true);
715+
});
716+
});
717+
575718
it("status icon should not toggle Errors panel when no errors present", function () {
576719
var codeInspector = createCodeInspector("javascript linter", successfulLintResult());
577720
CodeInspection.register("javascript", codeInspector);
@@ -810,7 +953,7 @@ define(function (require, exports, module) {
810953
});
811954
});
812955

813-
it("should report an async linter which throws an exception", function () {
956+
it("should report an async linter which rejects", function () {
814957
var errorMessage = "I'm full of bugs on purpose",
815958
providerName = "Buggy Async Linter",
816959
buggyAsyncProvider = {

0 commit comments

Comments
 (0)
⚡