From 7cf0b40280a80beb4309a3669b29f1ad509aeb04 Mon Sep 17 00:00:00 2001 From: Bryan Chin Date: Fri, 5 Sep 2014 11:06:35 -0500 Subject: [PATCH 01/12] fix Issue #8953- add Extension Manager support for localized extension package metadata. --- src/extensibility/ExtensionManagerView.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/extensibility/ExtensionManagerView.js b/src/extensibility/ExtensionManagerView.js index 40ab53b8f29..f5c00187bca 100644 --- a/src/extensibility/ExtensionManagerView.js +++ b/src/extensibility/ExtensionManagerView.js @@ -211,6 +211,19 @@ define(function (require, exports, module) { context.isCompatible = context.isCompatibleLatest = true; } + var lang = brackets.getLocale(), + shortLang = lang.split("-")[0]; + + // Extension metadata might have localized content. If so, overlay those strings. + if (info.metadata.hasOwnProperty(shortLang)) { + var prop; + for (prop in info.metadata[shortLang]) { + if (info.metadata[shortLang].hasOwnProperty(prop)) { + info.metadata[prop] = info.metadata[shortLang][prop]; + } + } + } + if (info.metadata.description !== undefined) { info.metadata.shortdescription = StringUtils.truncate(info.metadata.description, 200); } @@ -224,9 +237,6 @@ define(function (require, exports, module) { context.allowInstall = context.isCompatible && !context.isInstalled; if (Array.isArray(info.metadata.i18n) && info.metadata.i18n.length > 0) { - var lang = brackets.getLocale(), - shortLang = lang.split("-")[0]; - context.translated = true; context.translatedLangs = info.metadata.i18n.map(function (value) { From 8d476437c507a1c6f229cc97004dd07a684ac853 Mon Sep 17 00:00:00 2001 From: Bryan Chin Date: Fri, 5 Sep 2014 11:22:43 -0500 Subject: [PATCH 02/12] update localization example for reference --- src/extensions/samples/LocalizationExample/package.json | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/extensions/samples/LocalizationExample/package.json b/src/extensions/samples/LocalizationExample/package.json index 83b2020cb02..c6cca0099bb 100644 --- a/src/extensions/samples/LocalizationExample/package.json +++ b/src/extensions/samples/LocalizationExample/package.json @@ -12,5 +12,9 @@ "i18n": [ "en", "fr" - ] + ], + "fr": { + "title": "fr title to replace the root title", + "description": "fr description to replace the root description" + } } From d08d5571202092ed7cab39af8cbfd4c20b4a13f5 Mon Sep 17 00:00:00 2001 From: Bryan Chin Date: Fri, 5 Sep 2014 11:24:16 -0500 Subject: [PATCH 03/12] update localization readme for reference --- src/extensions/samples/LocalizationExample/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extensions/samples/LocalizationExample/README.md b/src/extensions/samples/LocalizationExample/README.md index 941199704a5..001e35d0a3d 100644 --- a/src/extensions/samples/LocalizationExample/README.md +++ b/src/extensions/samples/LocalizationExample/README.md @@ -48,7 +48,7 @@ Move this plugin to the extensions\user\ folder to run the plugin. It will add a * main.js – loads the Strings module for the plugin and uses mustache to localize html content -* package.json - add the translation languages as in the example: `"i18n: ["en", "fr" ]` +* package.json - add the translation languages as in the example: `"i18n: ["en", "fr" ]`. Also, add any localized metadata for displayed metadata in the Extension Manager, as in the example: `"fr": { "title": "localized title" }`. * strings.js – uses i18n to load a strings.js file in the nls folder From fbf682e6c86af14f4ed0905df27678cbffc2da21 Mon Sep 17 00:00:00 2001 From: Bryan Chin Date: Fri, 5 Sep 2014 19:13:52 -0500 Subject: [PATCH 04/12] as requested per code review --- src/extensibility/ExtensionManagerView.js | 26 +++++++++++++++-------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/extensibility/ExtensionManagerView.js b/src/extensibility/ExtensionManagerView.js index f5c00187bca..470ac5a1832 100644 --- a/src/extensibility/ExtensionManagerView.js +++ b/src/extensibility/ExtensionManagerView.js @@ -211,15 +211,23 @@ define(function (require, exports, module) { context.isCompatible = context.isCompatibleLatest = true; } - var lang = brackets.getLocale(), - shortLang = lang.split("-")[0]; - - // Extension metadata might have localized content. If so, overlay those strings. - if (info.metadata.hasOwnProperty(shortLang)) { - var prop; - for (prop in info.metadata[shortLang]) { - if (info.metadata[shortLang].hasOwnProperty(prop)) { - info.metadata[prop] = info.metadata[shortLang][prop]; + // Extension metadata might have localized content. If so, overlay (or concatenate) those strings. + var lang = brackets.getLocale(), + shortLang = lang.split("-")[0], + useShortLang = info.metadata.hasOwnProperty(shortLang); + if (useShortLang || info.metadata.hasOwnProperty(lang)) { + var key = useShortLang ? shortLang : lang, + prop; + for (prop in info.metadata[key]) { + if (info.metadata[key].hasOwnProperty(prop)) { + if (prop !== "keywords") { + // overlay existing string w/ localized string + info.metadata[prop] = info.metadata[key][prop]; + } else { + // for keywords, add the localized keywords to the root language keywords + var keywords = info.metadata[prop].concat(info.metadata[key][prop]); + info.metadata[prop] = keywords; + } } } } From dd6d57927ac0a1461726997ee61a72f53b16d6db Mon Sep 17 00:00:00 2001 From: Bryan Chin Date: Sat, 6 Sep 2014 00:47:40 -0500 Subject: [PATCH 05/12] changes per code review --- src/extensibility/ExtensionManagerView.js | 26 +++++++++++------------ 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/extensibility/ExtensionManagerView.js b/src/extensibility/ExtensionManagerView.js index 470ac5a1832..0bf217f8ed5 100644 --- a/src/extensibility/ExtensionManagerView.js +++ b/src/extensibility/ExtensionManagerView.js @@ -28,7 +28,8 @@ define(function (require, exports, module) { "use strict"; - var Strings = require("strings"), + var _ = require("thirdparty/lodash"), + Strings = require("strings"), StringUtils = require("utils/StringUtils"), ExtensionManager = require("extensibility/ExtensionManager"), registry_utils = require("extensibility/registry_utils"), @@ -216,20 +217,17 @@ define(function (require, exports, module) { shortLang = lang.split("-")[0], useShortLang = info.metadata.hasOwnProperty(shortLang); if (useShortLang || info.metadata.hasOwnProperty(lang)) { - var key = useShortLang ? shortLang : lang, - prop; - for (prop in info.metadata[key]) { - if (info.metadata[key].hasOwnProperty(prop)) { - if (prop !== "keywords") { - // overlay existing string w/ localized string - info.metadata[prop] = info.metadata[key][prop]; - } else { - // for keywords, add the localized keywords to the root language keywords - var keywords = info.metadata[prop].concat(info.metadata[key][prop]); - info.metadata[prop] = keywords; - } + var key = useShortLang ? shortLang : lang; + _.forEach(info.metadata[key], function (value, prop) { + if (prop !== "keywords") { + // overlay existing string w/ localized string + info.metadata[prop] = info.metadata[key][prop]; + } else { + // for keywords, add the localized keywords to the root language keywords + var keywords = info.metadata[prop].concat(info.metadata[key][prop]); + info.metadata[prop] = _.uniq(keywords); } - } + }); } if (info.metadata.description !== undefined) { From da86d2842bc7fd69ba1cc276d9d65862099a2b08 Mon Sep 17 00:00:00 2001 From: Bryan Chin Date: Tue, 9 Sep 2014 23:03:17 -0500 Subject: [PATCH 06/12] per code review, overlay first by shortlang, then by lang. --- src/extensibility/ExtensionManagerView.js | 33 ++++++++++++----------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/extensibility/ExtensionManagerView.js b/src/extensibility/ExtensionManagerView.js index 0bf217f8ed5..bbd2d842985 100644 --- a/src/extensibility/ExtensionManagerView.js +++ b/src/extensibility/ExtensionManagerView.js @@ -212,23 +212,24 @@ define(function (require, exports, module) { context.isCompatible = context.isCompatibleLatest = true; } - // Extension metadata might have localized content. If so, overlay (or concatenate) those strings. + // Extension metadata might have localized content. If so, overlay (or concatenate) those strings, first + // using any shortlang-specified values, and then the long language form. var lang = brackets.getLocale(), - shortLang = lang.split("-")[0], - useShortLang = info.metadata.hasOwnProperty(shortLang); - if (useShortLang || info.metadata.hasOwnProperty(lang)) { - var key = useShortLang ? shortLang : lang; - _.forEach(info.metadata[key], function (value, prop) { - if (prop !== "keywords") { - // overlay existing string w/ localized string - info.metadata[prop] = info.metadata[key][prop]; - } else { - // for keywords, add the localized keywords to the root language keywords - var keywords = info.metadata[prop].concat(info.metadata[key][prop]); - info.metadata[prop] = _.uniq(keywords); - } - }); - } + shortLang = lang.split("-")[0]; + [shortLang, lang].forEach(function (locale) { + if (info.metadata.hasOwnProperty(locale)) { + _.forEach(info.metadata[locale], function (value, prop) { + if (prop !== "keywords") { + // overlay existing string w/ localized string + info.metadata[prop] = value; + } else { + // for keywords, add the localized keywords to the root language keywords + var keywords = info.metadata[prop].concat(value); + info.metadata[prop] = _.uniq(keywords); + } + }); + } + }); if (info.metadata.description !== undefined) { info.metadata.shortdescription = StringUtils.truncate(info.metadata.description, 200); From cb01a983c263dbf35820a28499752e57d6fee3a9 Mon Sep 17 00:00:00 2001 From: Bryan Chin Date: Thu, 11 Sep 2014 22:14:05 -0500 Subject: [PATCH 07/12] whitelisting properties to overlay; remove keyword concatenation with root language --- src/extensibility/ExtensionManagerView.js | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/extensibility/ExtensionManagerView.js b/src/extensibility/ExtensionManagerView.js index 1fa535d7a1b..b5fd972932b 100644 --- a/src/extensibility/ExtensionManagerView.js +++ b/src/extensibility/ExtensionManagerView.js @@ -237,20 +237,15 @@ define(function (require, exports, module) { context.isCompatible = context.isCompatibleLatest = true; } - // Extension metadata might have localized content. If so, overlay (or concatenate) those strings, first - // using any shortlang-specified values, and then the long language form. + // Check if extension metadata contains localized content. var lang = brackets.getLocale(), shortLang = lang.split("-")[0]; [shortLang, lang].forEach(function (locale) { if (info.metadata.hasOwnProperty(locale)) { - _.forEach(info.metadata[locale], function (value, prop) { - if (prop !== "keywords") { - // overlay existing string w/ localized string - info.metadata[prop] = value; - } else { - // for keywords, add the localized keywords to the root language keywords - var keywords = info.metadata[prop].concat(value); - info.metadata[prop] = _.uniq(keywords); + // only overlay specific properties with the localized values + ['title', 'description', 'homepage', 'keywords'].forEach(function (prop) { + if (info.metadata[locale].hasOwnProperty(prop)) { + info.metadata[prop] = info.metadata[locale][prop]; } }); } From a7c823892f791a1a80d80d05d31bc764ecef7864 Mon Sep 17 00:00:00 2001 From: Bryan Chin Date: Thu, 11 Sep 2014 22:47:23 -0500 Subject: [PATCH 08/12] add top-level key to package.json to avoid collisions --- src/extensibility/ExtensionManagerView.js | 7 ++++--- src/extensions/samples/LocalizationExample/package.json | 8 +++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/extensibility/ExtensionManagerView.js b/src/extensibility/ExtensionManagerView.js index b5fd972932b..3ecae4c041d 100644 --- a/src/extensibility/ExtensionManagerView.js +++ b/src/extensibility/ExtensionManagerView.js @@ -241,11 +241,12 @@ define(function (require, exports, module) { var lang = brackets.getLocale(), shortLang = lang.split("-")[0]; [shortLang, lang].forEach(function (locale) { - if (info.metadata.hasOwnProperty(locale)) { + if (info.metadata["package-i18n"] !== undefined && + info.metadata["package-i18n"].hasOwnProperty(locale)) { // only overlay specific properties with the localized values ['title', 'description', 'homepage', 'keywords'].forEach(function (prop) { - if (info.metadata[locale].hasOwnProperty(prop)) { - info.metadata[prop] = info.metadata[locale][prop]; + if (info.metadata["package-i18n"][locale].hasOwnProperty(prop)) { + info.metadata[prop] = info.metadata["package-i18n"][locale][prop]; } }); } diff --git a/src/extensions/samples/LocalizationExample/package.json b/src/extensions/samples/LocalizationExample/package.json index c6cca0099bb..030295035d3 100644 --- a/src/extensions/samples/LocalizationExample/package.json +++ b/src/extensions/samples/LocalizationExample/package.json @@ -13,8 +13,10 @@ "en", "fr" ], - "fr": { - "title": "fr title to replace the root title", - "description": "fr description to replace the root description" + "package-i18n": { + "fr": { + "title": "Localisation Exemple", + "description": "Un guide sur la façon de localiser votre poste." + } } } From 117b20c777f8de6d31109cb46d91789266490f06 Mon Sep 17 00:00:00 2001 From: Bryan Chin Date: Mon, 15 Sep 2014 11:10:52 -0500 Subject: [PATCH 09/12] changes per code review --- src/extensibility/ExtensionManagerView.js | 26 +++++++++++------------ 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/extensibility/ExtensionManagerView.js b/src/extensibility/ExtensionManagerView.js index 3ecae4c041d..08c390e824c 100644 --- a/src/extensibility/ExtensionManagerView.js +++ b/src/extensibility/ExtensionManagerView.js @@ -28,8 +28,7 @@ define(function (require, exports, module) { "use strict"; - var _ = require("thirdparty/lodash"), - Strings = require("strings"), + var Strings = require("strings"), StringUtils = require("utils/StringUtils"), ExtensionManager = require("extensibility/ExtensionManager"), registry_utils = require("extensibility/registry_utils"), @@ -240,17 +239,18 @@ define(function (require, exports, module) { // Check if extension metadata contains localized content. var lang = brackets.getLocale(), shortLang = lang.split("-")[0]; - [shortLang, lang].forEach(function (locale) { - if (info.metadata["package-i18n"] !== undefined && - info.metadata["package-i18n"].hasOwnProperty(locale)) { - // only overlay specific properties with the localized values - ['title', 'description', 'homepage', 'keywords'].forEach(function (prop) { - if (info.metadata["package-i18n"][locale].hasOwnProperty(prop)) { - info.metadata[prop] = info.metadata["package-i18n"][locale][prop]; - } - }); - } - }); + if (info.metadata["package-i18n"]) { + [shortLang, lang].forEach(function (locale) { + if (info.metadata["package-i18n"].hasOwnProperty(locale)) { + // only overlay specific properties with the localized values + ['title', 'description', 'homepage', 'keywords'].forEach(function (prop) { + if (info.metadata["package-i18n"][locale].hasOwnProperty(prop)) { + info.metadata[prop] = info.metadata["package-i18n"][locale][prop]; + } + }); + } + }); + } if (info.metadata.description !== undefined) { info.metadata.shortdescription = StringUtils.truncate(info.metadata.description, 200); From 05d802df44b42ffe02d5f49980821d8f8e8fc218 Mon Sep 17 00:00:00 2001 From: Bryan Chin Date: Mon, 15 Sep 2014 12:34:03 -0500 Subject: [PATCH 10/12] adding unit tests --- .../mockRegistry.json | 11 +++- test/spec/ExtensionManager-test.js | 63 +++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/test/spec/ExtensionManager-test-files/mockRegistry.json b/test/spec/ExtensionManager-test-files/mockRegistry.json index b1b1054e768..259639f7071 100644 --- a/test/spec/ExtensionManager-test-files/mockRegistry.json +++ b/test/spec/ExtensionManager-test-files/mockRegistry.json @@ -48,7 +48,16 @@ "metadata": { "name": "mock-extension-1", "description": "First mock extension", - "version": "1.0.0" + "title": "Mock extension title", + "version": "1.0.0", + "i18n": [ "en", "fr" ], + "package-i18n": { + "fr": { + "description": "Première prolongation maquette", + "title": "Titre d'extension Mock", + "warnings": "Shouldn't be used" + } + } }, "owner": "github:mockuser", "versions": [ diff --git a/test/spec/ExtensionManager-test.js b/test/spec/ExtensionManager-test.js index 5fec9fc6055..c5d39dd5cda 100644 --- a/test/spec/ExtensionManager-test.js +++ b/test/spec/ExtensionManager-test.js @@ -1795,5 +1795,68 @@ define(function (require, exports, module) { }); }); }); + + describe("ExtensionManagerView-i18n", function () { + var view, model, fakeLoadDeferred, modelDisposed; + + // Sets up the view using the normal (mock) ExtensionManager data. + function setupViewWithMockData(ModelClass) { + runs(function () { + view = new ExtensionManagerView(); + model = new ModelClass(); + modelDisposed = false; + waitsForDone(view.initialize(model), "view initializing"); + view.$el.appendTo(document.body); + }); + runs(function () { + spyOn(view.model, "dispose").andCallThrough(); + }); + } + + beforeEach(function () { + this.addMatchers({ + toHaveText: function (expected) { + var notText = this.isNot ? " not" : ""; + this.message = function () { + return "Expected view" + notText + " to contain text " + expected; + }; + return SpecRunnerUtils.findDOMText(this.actual.$el, expected); + } + }); + spyOn(InstallExtensionDialog, "installUsingDialog").andCallFake(function (url) { + var id = url.match(/fake-repository\.com\/([^\/]+)/)[1]; + mockLoadExtensions(["user/" + id]); + }); + spyOn(brackets, "getLocale").andReturn("fr"); + }); + + + afterEach(function () { + if (view) { + view.$el.remove(); + view = null; + } + if (model) { + model.dispose(); + } + }); + + describe("when showing registry entries-i18n", function () { + it("should display localized description", function () { + setupViewWithMockData(ExtensionManagerViewModel.RegistryViewModel); + runs(function () { + _.forEach(mockRegistry, function (item) { + if (item.metadata["package-i18n"] && + item.metadata["package-i18n"].hasOwnProperty("fr") && + item.metadata["package-i18n"].fr.hasOwnProperty("description")) { + expect(view).toHaveText(item.metadata["package-i18n"].fr.description); + expect(view).toHaveText(item.metadata["package-i18n"].fr.title); + expect(view).not.toHaveText(item.metadata["package-i18n"].fr.warnings); + } + }); + }); + }); + }); + }); }); }); From 8fddc3863cf97d804d619a892b07e71021c13bc7 Mon Sep 17 00:00:00 2001 From: Bryan Chin Date: Thu, 18 Sep 2014 23:56:52 -0500 Subject: [PATCH 11/12] changes as requested per code review --- src/extensibility/ExtensionManagerView.js | 2 +- test/spec/ExtensionManager-test.js | 116 +++++++++------------- 2 files changed, 50 insertions(+), 68 deletions(-) diff --git a/src/extensibility/ExtensionManagerView.js b/src/extensibility/ExtensionManagerView.js index 08c390e824c..cf7396f7315 100644 --- a/src/extensibility/ExtensionManagerView.js +++ b/src/extensibility/ExtensionManagerView.js @@ -243,7 +243,7 @@ define(function (require, exports, module) { [shortLang, lang].forEach(function (locale) { if (info.metadata["package-i18n"].hasOwnProperty(locale)) { // only overlay specific properties with the localized values - ['title', 'description', 'homepage', 'keywords'].forEach(function (prop) { + ["title", "description", "homepage", "keywords"].forEach(function (prop) { if (info.metadata["package-i18n"][locale].hasOwnProperty(prop)) { info.metadata[prop] = info.metadata["package-i18n"][locale][prop]; } diff --git a/test/spec/ExtensionManager-test.js b/test/spec/ExtensionManager-test.js index c5d39dd5cda..96973c20d5b 100644 --- a/test/spec/ExtensionManager-test.js +++ b/test/spec/ExtensionManager-test.js @@ -58,7 +58,8 @@ define(function (require, exports, module) { mockRegistry; describe("ExtensionManager", function () { - var mockId, mockSettings, origRegistryURL, origExtensionUrl, removedPath; + var mockId, mockSettings, origRegistryURL, origExtensionUrl, removedPath, + view, model, fakeLoadDeferred, modelDisposed; beforeEach(function () { // Use fake URLs for the registry (useful if the registry isn't actually currently @@ -158,6 +159,37 @@ define(function (require, exports, module) { }; } + function setupExtensionManagerViewTests() { + spyOn(InstallExtensionDialog, "installUsingDialog").andCallFake(function (url) { + var id = url.match(/fake-repository\.com\/([^\/]+)/)[1]; + mockLoadExtensions(["user/" + id]); + }); + } + + function cleanupExtensionManagerViewTests() { + if (view) { + view.$el.remove(); + view = null; + } + if (model) { + model.dispose(); + } + } + + // Sets up the view using the normal (mock) ExtensionManager data. + function setupViewWithMockData(ModelClass) { + runs(function () { + view = new ExtensionManagerView(); + model = new ModelClass(); + modelDisposed = false; + waitsForDone(view.initialize(model), "view initializing"); + view.$el.appendTo(document.body); + }); + runs(function () { + spyOn(view.model, "dispose").andCallThrough(); + }); + } + describe("ExtensionManager", function () { it("should download the extension list from the registry", function () { runs(function () { @@ -862,22 +894,7 @@ define(function (require, exports, module) { }); describe("ExtensionManagerView", function () { - var view, model, fakeLoadDeferred, modelDisposed; - - // Sets up the view using the normal (mock) ExtensionManager data. - function setupViewWithMockData(ModelClass) { - runs(function () { - view = new ExtensionManagerView(); - model = new ModelClass(); - modelDisposed = false; - waitsForDone(view.initialize(model), "view initializing"); - view.$el.appendTo(document.body); - }); - runs(function () { - spyOn(view.model, "dispose").andCallThrough(); - }); - } - + beforeEach(function () { this.addMatchers({ toHaveText: function (expected) { @@ -895,22 +912,13 @@ define(function (require, exports, module) { return SpecRunnerUtils.findDOMText(this.actual.$el, expected, true); } }); - spyOn(InstallExtensionDialog, "installUsingDialog").andCallFake(function (url) { - var id = url.match(/fake-repository\.com\/([^\/]+)/)[1]; - mockLoadExtensions(["user/" + id]); - }); + setupExtensionManagerViewTests(); spyOn(brackets, "getLocale").andReturn("en"); }); afterEach(function () { - if (view) { - view.$el.remove(); - view = null; - } - if (model) { - model.dispose(); - } + cleanupExtensionManagerViewTests(); }); describe("when showing registry entries", function () { @@ -1797,21 +1805,6 @@ define(function (require, exports, module) { }); describe("ExtensionManagerView-i18n", function () { - var view, model, fakeLoadDeferred, modelDisposed; - - // Sets up the view using the normal (mock) ExtensionManager data. - function setupViewWithMockData(ModelClass) { - runs(function () { - view = new ExtensionManagerView(); - model = new ModelClass(); - modelDisposed = false; - waitsForDone(view.initialize(model), "view initializing"); - view.$el.appendTo(document.body); - }); - runs(function () { - spyOn(view.model, "dispose").andCallThrough(); - }); - } beforeEach(function () { this.addMatchers({ @@ -1823,37 +1816,26 @@ define(function (require, exports, module) { return SpecRunnerUtils.findDOMText(this.actual.$el, expected); } }); - spyOn(InstallExtensionDialog, "installUsingDialog").andCallFake(function (url) { - var id = url.match(/fake-repository\.com\/([^\/]+)/)[1]; - mockLoadExtensions(["user/" + id]); - }); + setupExtensionManagerViewTests(); spyOn(brackets, "getLocale").andReturn("fr"); }); afterEach(function () { - if (view) { - view.$el.remove(); - view = null; - } - if (model) { - model.dispose(); - } + cleanupExtensionManagerViewTests(); }); - describe("when showing registry entries-i18n", function () { - it("should display localized description", function () { - setupViewWithMockData(ExtensionManagerViewModel.RegistryViewModel); - runs(function () { - _.forEach(mockRegistry, function (item) { - if (item.metadata["package-i18n"] && - item.metadata["package-i18n"].hasOwnProperty("fr") && - item.metadata["package-i18n"].fr.hasOwnProperty("description")) { - expect(view).toHaveText(item.metadata["package-i18n"].fr.description); - expect(view).toHaveText(item.metadata["package-i18n"].fr.title); - expect(view).not.toHaveText(item.metadata["package-i18n"].fr.warnings); - } - }); + it("should display localized description", function () { + setupViewWithMockData(ExtensionManagerViewModel.RegistryViewModel); + runs(function () { + _.forEach(mockRegistry, function (item) { + if (item.metadata["package-i18n"] && + item.metadata["package-i18n"].hasOwnProperty("fr") && + item.metadata["package-i18n"].fr.hasOwnProperty("description")) { + expect(view).toHaveText(item.metadata["package-i18n"].fr.description); + expect(view).toHaveText(item.metadata["package-i18n"].fr.title); + expect(view).not.toHaveText(item.metadata["package-i18n"].fr.warnings); + } }); }); }); From c9734474849d85126bc850f766e33414d632ea48 Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Fri, 19 Sep 2014 01:22:06 -0700 Subject: [PATCH 12/12] Further consolidate duplicated code in a unit test --- test/spec/ExtensionManager-test.js | 48 ++++++++++++------------------ 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/test/spec/ExtensionManager-test.js b/test/spec/ExtensionManager-test.js index 96973c20d5b..2eece419f6f 100644 --- a/test/spec/ExtensionManager-test.js +++ b/test/spec/ExtensionManager-test.js @@ -159,7 +159,23 @@ define(function (require, exports, module) { }; } - function setupExtensionManagerViewTests() { + function setupExtensionManagerViewTests(context) { + context.addMatchers({ + toHaveText: function (expected) { + var notText = this.isNot ? " not" : ""; + this.message = function () { + return "Expected view" + notText + " to contain text " + expected; + }; + return SpecRunnerUtils.findDOMText(this.actual.$el, expected); + }, + toHaveLink: function (expected) { + var notText = this.isNot ? " not" : ""; + this.message = function () { + return "Expected view" + notText + " to contain link " + expected; + }; + return SpecRunnerUtils.findDOMText(this.actual.$el, expected, true); + } + }); spyOn(InstallExtensionDialog, "installUsingDialog").andCallFake(function (url) { var id = url.match(/fake-repository\.com\/([^\/]+)/)[1]; mockLoadExtensions(["user/" + id]); @@ -896,23 +912,7 @@ define(function (require, exports, module) { describe("ExtensionManagerView", function () { beforeEach(function () { - this.addMatchers({ - toHaveText: function (expected) { - var notText = this.isNot ? " not" : ""; - this.message = function () { - return "Expected view" + notText + " to contain text " + expected; - }; - return SpecRunnerUtils.findDOMText(this.actual.$el, expected); - }, - toHaveLink: function (expected) { - var notText = this.isNot ? " not" : ""; - this.message = function () { - return "Expected view" + notText + " to contain link " + expected; - }; - return SpecRunnerUtils.findDOMText(this.actual.$el, expected, true); - } - }); - setupExtensionManagerViewTests(); + setupExtensionManagerViewTests(this); spyOn(brackets, "getLocale").andReturn("en"); }); @@ -1807,20 +1807,10 @@ define(function (require, exports, module) { describe("ExtensionManagerView-i18n", function () { beforeEach(function () { - this.addMatchers({ - toHaveText: function (expected) { - var notText = this.isNot ? " not" : ""; - this.message = function () { - return "Expected view" + notText + " to contain text " + expected; - }; - return SpecRunnerUtils.findDOMText(this.actual.$el, expected); - } - }); - setupExtensionManagerViewTests(); + setupExtensionManagerViewTests(this); spyOn(brackets, "getLocale").andReturn("fr"); }); - afterEach(function () { cleanupExtensionManagerViewTests(); });