diff --git a/spec/config-spec.coffee b/spec/config-spec.coffee index 9640fae3e..4940678fe 100644 --- a/spec/config-spec.coffee +++ b/spec/config-spec.coffee @@ -493,13 +493,19 @@ describe "Config", -> expect(observeHandler).not.toHaveBeenCalled() it "fires the callback every time the observed value changes", -> - observeHandler.reset() # clear the initial call atom.config.set('foo.bar.baz', "value 2") expect(observeHandler).toHaveBeenCalledWith({newValue: 'value 2', oldValue: 'value 1'}) observeHandler.reset() - atom.config.set('foo.bar.baz', "value 1") + observeHandler.andCallFake -> throw new Error("oops") + expect(-> atom.config.set('foo.bar.baz', "value 1")).toThrow("oops") expect(observeHandler).toHaveBeenCalledWith({newValue: 'value 1', oldValue: 'value 2'}) + observeHandler.reset() + + # Regression: exception in earlier handler shouldn't put observer + # into a bad state. + atom.config.set('something.else', "new value") + expect(observeHandler).not.toHaveBeenCalled() describe 'when a keyPath is not specified', -> beforeEach -> @@ -567,7 +573,7 @@ describe "Config", -> atom.config.set("foo.bar.baz", "i'm back") expect(observeHandler).toHaveBeenCalledWith("i'm back") - it "does not fire the callback once the observe subscription is off'ed", -> + it "does not fire the callback once the subscription is disposed", -> observeHandler.reset() # clear the initial call observeSubscription.dispose() atom.config.set('foo.bar.baz', "value 2") diff --git a/spec/package-manager-spec.coffee b/spec/package-manager-spec.coffee index bac6270f8..2eee6c0d0 100644 --- a/spec/package-manager-spec.coffee +++ b/spec/package-manager-spec.coffee @@ -3,19 +3,36 @@ Package = require '../src/package' describe "PackageManager", -> workspaceElement = null + beforeEach -> workspaceElement = atom.views.getView(atom.workspace) describe "::loadPackage(name)", -> - it "continues if the package has an invalid package.json", -> - spyOn(console, 'warn') + beforeEach -> atom.config.set("core.disabledPackages", []) - expect(-> atom.packages.loadPackage("package-with-broken-package-json")).not.toThrow() - it "continues if the package has an invalid keymap", -> + it "returns the package", -> + pack = atom.packages.loadPackage("package-with-index") + expect(pack instanceof Package).toBe true + expect(pack.metadata.name).toBe "package-with-index" + + it "returns the package if it has an invalid keymap", -> spyOn(console, 'warn') - atom.config.set("core.disabledPackages", []) - expect(-> atom.packages.loadPackage("package-with-broken-keymap")).not.toThrow() + pack = atom.packages.loadPackage("package-with-broken-keymap") + expect(pack instanceof Package).toBe true + expect(pack.metadata.name).toBe "package-with-broken-keymap" + + it "returns null if the package has an invalid package.json", -> + spyOn(console, 'warn') + expect(atom.packages.loadPackage("package-with-broken-package-json")).toBeNull() + expect(console.warn.callCount).toBe(1) + expect(console.warn.argsForCall[0][0]).toContain("Failed to load package.json") + + it "returns null if the package is not found in any package directory", -> + spyOn(console, 'warn') + expect(atom.packages.loadPackage("this-package-cannot-be-found")).toBeNull() + expect(console.warn.callCount).toBe(1) + expect(console.warn.argsForCall[0][0]).toContain("Could not resolve") describe "::unloadPackage(name)", -> describe "when the package is active", -> @@ -197,11 +214,30 @@ describe "PackageManager", -> runs -> expect(pack.mainModule.activate).toHaveBeenCalledWith({someNumber: 77}) - it "logs warning instead of throwing an exception if the package fails to load", -> - atom.config.set("core.disabledPackages", []) - spyOn(console, "warn") - expect(-> atom.packages.activatePackage("package-that-throws-an-exception")).not.toThrow() - expect(console.warn).toHaveBeenCalled() + describe "when the package throws an error while loading", -> + it "logs a warning instead of throwing an exception", -> + atom.config.set("core.disabledPackages", []) + spyOn(console, "warn") + expect(-> atom.packages.activatePackage("package-that-throws-an-exception")).not.toThrow() + expect(console.warn).toHaveBeenCalled() + + describe "when the package is not found", -> + it "rejects the promise", -> + atom.config.set("core.disabledPackages", []) + + onSuccess = jasmine.createSpy('onSuccess') + onFailure = jasmine.createSpy('onFailure') + spyOn(console, 'warn') + + atom.packages.activatePackage("this-doesnt-exist").then(onSuccess, onFailure) + + waitsFor "promise to be rejected", -> + onFailure.callCount > 0 + + runs -> + expect(console.warn.callCount).toBe 1 + expect(onFailure.mostRecentCall.args[0] instanceof Error).toBe true + expect(onFailure.mostRecentCall.args[0].message).toContain "Failed to load package 'this-doesnt-exist'" describe "keymap loading", -> describe "when the metadata does not contain a 'keymaps' manifest", -> @@ -552,9 +588,9 @@ describe "PackageManager", -> themes = themeActivator.mostRecentCall.args[0] expect(['theme']).toContain(theme.getType()) for theme in themes - describe "::enablePackage() and ::disablePackage()", -> + describe "::enablePackage(id) and ::disablePackage(id)", -> describe "with packages", -> - it ".enablePackage() enables a disabled package", -> + it "enables a disabled package", -> packageName = 'package-with-main' atom.config.pushAtKeyPath('core.disabledPackages', packageName) atom.packages.observeDisabledPackages() @@ -572,7 +608,7 @@ describe "PackageManager", -> expect(activatedPackages).toContain(pack) expect(atom.config.get('core.disabledPackages')).not.toContain packageName - it ".disablePackage() disables an enabled package", -> + it "disables an enabled package", -> packageName = 'package-with-main' waitsForPromise -> atom.packages.activatePackage(packageName) @@ -587,6 +623,11 @@ describe "PackageManager", -> expect(activatedPackages).not.toContain(pack) expect(atom.config.get('core.disabledPackages')).toContain packageName + it "returns null if the package cannot be loaded", -> + spyOn(console, 'warn') + expect(atom.packages.enablePackage("this-doesnt-exist")).toBeNull() + expect(console.warn.callCount).toBe 1 + describe "with themes", -> reloadedHandler = null @@ -597,7 +638,7 @@ describe "PackageManager", -> afterEach -> atom.themes.deactivateThemes() - it ".enablePackage() and .disablePackage() enables and disables a theme", -> + it "enables and disables a theme", -> packageName = 'theme-with-package-file' expect(atom.config.get('core.themes')).not.toContain packageName diff --git a/spec/theme-manager-spec.coffee b/spec/theme-manager-spec.coffee index a5660ff61..dffec95b7 100644 --- a/spec/theme-manager-spec.coffee +++ b/spec/theme-manager-spec.coffee @@ -131,7 +131,9 @@ describe "ThemeManager", -> describe "when a theme fails to load", -> it "logs a warning", -> spyOn(console, 'warn') - expect(-> atom.packages.activatePackage('a-theme-that-will-not-be-found')).toThrow() + atom.packages.activatePackage('a-theme-that-will-not-be-found') + expect(console.warn.callCount).toBe 1 + expect(console.warn.argsForCall[0][0]).toContain "Could not resolve 'a-theme-that-will-not-be-found'" describe "::requireStylesheet(path)", -> beforeEach -> diff --git a/src/config.coffee b/src/config.coffee index d0d8f7d2f..9db755f2f 100644 --- a/src/config.coffee +++ b/src/config.coffee @@ -882,13 +882,12 @@ class Config onDidChangeKeyPath: (keyPath, callback) -> oldValue = @get(keyPath) - - didChange = => + @emitter.on 'did-change', => newValue = @get(keyPath) - callback({oldValue, newValue}) unless _.isEqual(oldValue, newValue) - oldValue = newValue - - @emitter.on 'did-change', didChange + unless _.isEqual(oldValue, newValue) + event = {oldValue, newValue} + oldValue = newValue + callback(event) isSubKeyPath: (keyPath, subKeyPath) -> return false unless keyPath? and subKeyPath? @@ -1003,12 +1002,12 @@ class Config onDidChangeScopedKeyPath: (scope, keyPath, callback) -> oldValue = @get(keyPath, {scope}) - didChange = => + @emitter.on 'did-change', => newValue = @get(keyPath, {scope}) - callback({oldValue, newValue}) unless _.isEqual(oldValue, newValue) - oldValue = newValue - - @emitter.on 'did-change', didChange + unless _.isEqual(oldValue, newValue) + event = {oldValue, newValue} + oldValue = newValue + callback(event) # TODO: figure out how to change / remove this. The return value is awkward. # * language mode uses it for one thing. diff --git a/src/package-manager.coffee b/src/package-manager.coffee index 31b9046a2..6edad3007 100644 --- a/src/package-manager.coffee +++ b/src/package-manager.coffee @@ -295,12 +295,12 @@ class PackageManager pack = new Package(packagePath, metadata) pack.load() @loadedPackages[pack.name] = pack - pack + return pack catch error console.warn "Failed to load package.json '#{path.basename(packagePath)}'", error.stack ? error - else - throw new Error("Could not resolve '#{nameOrPath}' to a package path") + console.warn "Could not resolve '#{nameOrPath}' to a package path" + null unloadPackages: -> @unloadPackage(name) for name in _.keys(@loadedPackages) @@ -337,11 +337,12 @@ class PackageManager activatePackage: (name) -> if pack = @getActivePackage(name) Q(pack) - else - pack = @loadPackage(name) + else if pack = @loadPackage(name) pack.activate().then => @activePackages[pack.name] = pack pack + else + Q.reject(new Error("Failed to load package '#{name}'")) # Deactivate all packages deactivatePackages: ->