fix(specs): silence noisy specs and fix warnings

Summary:
Silence buffered process spec

Clean up error reporter and spec bootup

fix errors in draft store spec

package manager spec and theme spec fixes

Fix memory leak in draft store

Test Plan: mmmmmm tests. Run all those green passing tests :)

Reviewers: bengotow

Reviewed By: bengotow

Differential Revision: https://phab.nylas.com/D1628
This commit is contained in:
Evan Morikawa 2015-06-15 18:29:59 -07:00
parent f6b5b7ea30
commit 304c34f918
14 changed files with 320 additions and 247 deletions

View file

@ -933,6 +933,8 @@ class ContenteditableComponent extends React.Component
document.execCommand("insertHTML", false, cleanHtml)
@_selectionManuallyChanged = true
return
# This is used primarily when pasting text in
_sanitizeInput: (inputText="", type="text/html") =>
if type is "text/plain"

View file

@ -14,13 +14,13 @@ describe "BufferedProcess", ->
describe "when there is an error handler specified", ->
it "calls the error handler and does not throw an exception", ->
process = new BufferedProcess
p = new BufferedProcess
command: 'bad-command-nope'
args: ['nothing']
options: {}
errorSpy = jasmine.createSpy().andCallFake (error) -> error.handle()
process.onWillThrowError(errorSpy)
p.onWillThrowError(errorSpy)
waitsFor -> errorSpy.callCount > 0
@ -31,17 +31,17 @@ describe "BufferedProcess", ->
describe "when there is not an error handler specified", ->
it "calls the error handler and does not throw an exception", ->
process = new BufferedProcess
spyOn(process, "nextTick").andCallFake (fn) -> fn()
try
p = new BufferedProcess
command: 'bad-command-nope'
args: ['nothing']
options: {}
options: {stdout: 'ignore'}
waitsFor -> window.onerror.callCount > 0
runs ->
expect(window.onerror).toHaveBeenCalled()
expect(window.onerror.mostRecentCall.args[0]).toContain 'Failed to spawn command `bad-command-nope`'
expect(window.onerror.mostRecentCall.args[4].name).toBe 'BufferedProcessError'
catch error
expect(error.message).toContain 'Failed to spawn command `bad-command-nope`'
expect(error.name).toBe 'BufferedProcessError'
describe "on Windows", ->
originalPlatform = null

View file

@ -201,6 +201,7 @@ describe "NylasSyncWorker", ->
expect(@connection.end).toHaveBeenCalled()
it "should stop trying to restart failed collection syncs", ->
spyOn(console, 'log')
spyOn(@worker, 'resumeFetches').andCallThrough()
@worker.cleanup()
advanceClock(50000)

View file

@ -1,3 +1,4 @@
path = require 'path'
{$, $$} = require '../src/space-pen-extensions'
Package = require '../src/package'
{Disposable} = require 'atom'
@ -20,11 +21,14 @@ describe "PackageManager", ->
it "returns the package if it has an invalid keymap", ->
spyOn(console, 'warn')
spyOn(console, 'error')
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 the package if it has an invalid stylesheet", ->
spyOn(console, 'warn')
spyOn(console, 'error')
pack = atom.packages.loadPackage("package-with-invalid-styles")
expect(pack instanceof Package).toBe true
expect(pack.metadata.name).toBe "package-with-invalid-styles"
@ -32,12 +36,14 @@ describe "PackageManager", ->
it "returns null if the package has an invalid package.json", ->
spyOn(console, 'warn')
spyOn(console, 'error')
expect(atom.packages.loadPackage("package-with-broken-package-json")).toBeNull()
expect(console.warn.callCount).toBe(2)
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')
spyOn(console, 'error')
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")
@ -158,7 +164,7 @@ describe "PackageManager", ->
describe "when the package has no main module", ->
it "does not throw an exception", ->
spyOn(console, "error")
spyOn(console, "warn").andCallThrough()
spyOn(console, "warn")
expect(-> atom.packages.activatePackage('package-without-module')).not.toThrow()
expect(console.error).not.toHaveBeenCalled()
expect(console.warn).not.toHaveBeenCalled()
@ -191,7 +197,9 @@ describe "PackageManager", ->
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, "log")
spyOn(console, "warn")
spyOn(console, "error")
expect(-> atom.packages.activatePackage("package-that-throws-an-exception")).not.toThrow()
expect(console.warn).toHaveBeenCalled()
@ -202,6 +210,7 @@ describe "PackageManager", ->
onSuccess = jasmine.createSpy('onSuccess')
onFailure = jasmine.createSpy('onFailure')
spyOn(console, 'warn')
spyOn(console, "error")
atom.packages.activatePackage("this-doesnt-exist").then(onSuccess, onFailure)
@ -433,7 +442,9 @@ describe "PackageManager", ->
expect(pack.mainModule.deactivate).toHaveBeenCalled()
expect(atom.packages.isPackageActive("package-with-module")).toBeFalsy()
spyOn(console, 'log')
spyOn(console, 'warn')
spyOn(console, "error")
badPack = null
waitsForPromise ->
@ -448,7 +459,9 @@ describe "PackageManager", ->
expect(atom.packages.isPackageActive("package-that-throws-on-activate")).toBeFalsy()
it "does not serialize packages that have not been activated called on their main module", ->
spyOn(console, 'log')
spyOn(console, 'warn')
spyOn(console, "error")
badPack = null
waitsForPromise ->
atom.packages.activatePackage("package-that-throws-on-activate").then (p) -> badPack = p
@ -529,6 +542,7 @@ describe "PackageManager", ->
beforeEach ->
jasmine.snapshotDeprecations()
spyOn(console, 'warn')
spyOn(console, "error")
atom.packages.loadPackages()
loadedPackages = atom.packages.getLoadedPackages()
@ -608,6 +622,7 @@ describe "PackageManager", ->
it "returns null if the package cannot be loaded", ->
spyOn(console, 'warn')
spyOn(console, "error")
expect(atom.packages.enablePackage("this-doesnt-exist")).toBeNull()
expect(console.warn.callCount).toBe 1
@ -615,6 +630,8 @@ describe "PackageManager", ->
didChangeActiveThemesHandler = null
beforeEach ->
theme_dir = path.resolve(__dirname, '../internal_packages')
atom.packages.packageDirPaths.unshift(theme_dir)
waitsForPromise ->
atom.themes.activateThemes()

View file

@ -85,9 +85,15 @@ describe "DraftStore", ->
spyOn(DatabaseStore, 'run').andCallFake (query) ->
return Promise.resolve(fakeMessage2) if query._klass is Message
return Promise.reject(new Error('Not Stubbed'))
spyOn(DatabaseStore, 'persistModel')
spyOn(DatabaseStore, 'persistModel').andCallFake -> Promise.resolve()
spyOn(DatabaseStore, 'bindToLocalId')
afterEach ->
# Have to cleanup the DraftStoreProxy objects or we'll get a memory
# leak error
for id, session of DraftStore._draftSessions
DraftStore._doneWithSession(session)
describe "onComposeReply", ->
beforeEach ->
runs ->

View file

@ -13,6 +13,11 @@ describe "ThemeManager", ->
configDirPath = atom.getConfigDirPath()
beforeEach ->
spyOn(console, "log")
spyOn(console, "warn")
spyOn(console, "error")
theme_dir = path.resolve(__dirname, '../internal_packages')
atom.packages.packageDirPaths.unshift(theme_dir)
themeManager = new ThemeManager({packageManager: atom.packages, resourcePath, configDirPath})
afterEach ->
@ -153,7 +158,6 @@ describe "ThemeManager", ->
describe "when a theme fails to load", ->
it "logs a warning", ->
spyOn(console, 'warn')
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'"
@ -381,7 +385,6 @@ describe "ThemeManager", ->
themeManager.loadUserStylesheet()
spyOn(themeManager.lessCache, 'cssForFile').andCallFake ->
throw new Error('EACCES permission denied "styles.less"')
spyOn(console, 'error').andCallThrough()
it "creates an error notification and does not add the stylesheet", ->
themeManager.loadUserStylesheet()
@ -397,7 +400,6 @@ describe "ThemeManager", ->
spyOn(File::, 'onDidChange').andCallFake (event) ->
throw new Error('Unable to watch path')
spyOn(themeManager, 'loadStylesheet').andReturn ''
spyOn(console, 'error').andCallThrough()
it "creates an error notification", ->
themeManager.loadUserStylesheet()
@ -407,7 +409,6 @@ describe "ThemeManager", ->
describe "when a non-existent theme is present in the config", ->
beforeEach ->
spyOn(console, 'warn')
atom.config.set('core.themes', ['non-existent-dark-ui'])
waitsForPromise ->

View file

@ -27,8 +27,6 @@ atom.themes.loadBaseStylesheets()
atom.themes.requireStylesheet '../static/jasmine'
atom.themes.initialLoadComplete = true
fixturePackagesPath = path.resolve(__dirname, '../spec-nylas/fixtures/packages')
atom.packages.packageDirPaths.unshift(fixturePackagesPath)
atom.keymaps.loadBundledKeymaps()
keyBindingsToRestore = atom.keymaps.getKeyBindings()
commandsToRestore = atom.commands.getSnapshot()
@ -172,7 +170,20 @@ beforeEach ->
addCustomMatchers(this)
original_log = console.log
original_warn = console.warn
original_error = console.error
afterEach ->
if console.log isnt original_log
console.log = original_log
if console.warn isnt original_warn
console.warn = original_warn
if console.error isnt original_error
console.error = original_error
atom.packages.deactivatePackages()
atom.menu.template = []

View file

@ -205,7 +205,8 @@ class Atom extends Model
@actionBridge = new ActionBridge(ipc)
@commands = new CommandRegistry
@packages = new PackageManager({devMode, configDirPath, resourcePath, safeMode})
specMode = @inSpecMode()
@packages = new PackageManager({devMode, configDirPath, resourcePath, safeMode, specMode})
@styles = new StyleManager
document.head.appendChild(new StylesElement)
@themes = new ThemeManager({packageManager: @packages, configDirPath, resourcePath, safeMode})
@ -229,7 +230,10 @@ class Atom extends Model
# back through the sourcemap as necessary.
setupErrorHandling: ->
ErrorReporter = require './error-reporter'
@errorReporter = new ErrorReporter()
@errorReporter = new ErrorReporter
inSpecMode: @inSpecMode()
inDevMode: @inDevMode()
sourceMapCache = {}
window.onerror = =>

View file

@ -65,7 +65,8 @@ class Application
exit: (status) -> app.exit(status)
constructor: (options) ->
{@resourcePath, @version, @devMode, @safeMode} = options
{@resourcePath, @version, @devMode, test, @safeMode} = options
@specMode = test
# Normalize to make sure drive letter case is consistent on Windows
@resourcePath = path.normalize(@resourcePath) if @resourcePath
@ -77,7 +78,7 @@ class Application
@databases = {}
@windowManager = new WindowManager({@resourcePath, @config, @devMode, @safeMode})
@autoUpdateManager = new AutoUpdateManager(@version, @config)
@autoUpdateManager = new AutoUpdateManager(@version, @config, @specMode)
@applicationMenu = new ApplicationMenu(@version)
@nylasProtocolHandler = new NylasProtocolHandler(@resourcePath, @safeMode)

View file

@ -16,7 +16,7 @@ module.exports =
class AutoUpdateManager
_.extend @prototype, EventEmitter.prototype
constructor: (@version, @config) ->
constructor: (@version, @config, @specMode) ->
@state = IdleState
if process.platform is 'win32'
# Squirrel for Windows can't handle query params
@ -26,6 +26,7 @@ class AutoUpdateManager
upgradeLevel = @getUpgradeLevel()
@feedUrl = "https://edgehill.nylas.com/update-check?version=#{@version}&level=#{upgradeLevel}"
if not @specMode
process.nextTick => @setupAutoUpdater()
getUpgradeLevel: ->

View file

@ -1,20 +1,20 @@
global.shellStartTime = Date.now()
errorReporter = new (require '../error-reporter')
app = require 'app'
fs = require 'fs'
path = require 'path'
optimist = require 'optimist'
start = ->
args = parseCommandLine()
global.errorReporter = setupErrorReporter(args)
if process.platform is 'win32'
SquirrelUpdate = require './squirrel-update'
squirrelCommand = process.argv[1]
return if SquirrelUpdate.handleStartupEvent(app, squirrelCommand)
args = parseCommandLine()
addPathToOpen = (event, pathToOpen) ->
event.preventDefault()
args.pathsToOpen.push(pathToOpen)
@ -56,10 +56,14 @@ global.devResourcePath = process.env.EDGEHILL_PATH ? process.cwd()
# Normalize to make sure drive letter case is consistent on Windows
global.devResourcePath = path.normalize(global.devResourcePath) if global.devResourcePath
setupErrorReporter = (args={}) ->
ErrorReporter = require '../error-reporter'
return new ErrorReporter({inSpecMode: args.test, inDevMode: args.devMode})
setupCrashReporter = ->
# In the future, we may want to collect actual native crash reports,
# but for now let's not send them to github.
# crashReporter.start(productName: 'Atom', companyName: 'GitHub')
# but for now let's not send them to GitHub
# crashReporter.start(productName: "Nylas Mail", companyName: "Nylas")
setupCoffeeScript = ->
CoffeeScript = null

View file

@ -1,9 +1,9 @@
var app, errorReporter, fs, lstatSyncNoException, optimist, parseCommandLine, path, setupCoffeeScript, setupCrashReporter, start, statSyncNoException, _ref;
// Generated by CoffeeScript 1.9.2
(function() {
var app, fs, optimist, parseCommandLine, path, ref, setupCoffeeScript, setupErrorReporter, start;
global.shellStartTime = Date.now();
global.errorReporter = new (require('../error-reporter'));
app = require('app');
fs = require('fs');
@ -12,24 +12,10 @@ path = require('path');
optimist = require('optimist');
lstatSyncNoException = fs.lstatSyncNoException, statSyncNoException = fs.statSyncNoException;
fs.statSyncNoException = function(pathToStat) {
if (!(pathToStat && typeof pathToStat === 'string')) {
return false;
}
return statSyncNoException(pathToStat);
};
fs.lstatSyncNoException = function(pathToStat) {
if (!(pathToStat && typeof pathToStat === 'string')) {
return false;
}
return lstatSyncNoException(pathToStat);
};
start = function() {
var SquirrelUpdate, addPathToOpen, addUrlToOpen, args, squirrelCommand;
args = parseCommandLine();
global.errorReporter = setupErrorReporter(args);
if (process.platform === 'win32') {
SquirrelUpdate = require('./squirrel-update');
squirrelCommand = process.argv[1];
@ -37,7 +23,6 @@ start = function() {
return;
}
}
args = parseCommandLine();
addPathToOpen = function(event, pathToOpen) {
event.preventDefault();
return args.pathsToOpen.push(pathToOpen);
@ -49,16 +34,17 @@ start = function() {
};
app.on('open-file', addPathToOpen);
app.on('open-url', addUrlToOpen);
app.on('will-finish-launching', function() {
return setupCrashReporter();
});
return app.on('ready', function() {
var Application;
var Application, cwd, ref;
app.removeListener('open-file', addPathToOpen);
app.removeListener('open-url', addUrlToOpen);
cwd = ((ref = args.executedFrom) != null ? ref.toString() : void 0) || process.cwd();
args.pathsToOpen = args.pathsToOpen.map(function(pathToOpen) {
var _ref;
return path.resolve((_ref = args.executedFrom) != null ? _ref : process.cwd(), pathToOpen.toString());
if (cwd) {
return path.resolve(cwd, pathToOpen.toString());
} else {
return path.resolve(pathToOpen.toString());
}
});
setupCoffeeScript();
if (args.devMode) {
@ -74,13 +60,23 @@ start = function() {
});
};
global.devResourcePath = (_ref = process.env.EDGEHILL_PATH) != null ? _ref : process.cwd();
global.devResourcePath = (ref = process.env.EDGEHILL_PATH) != null ? ref : process.cwd();
if (global.devResourcePath) {
global.devResourcePath = path.normalize(global.devResourcePath);
}
setupCrashReporter = function() {};
setupErrorReporter = function(args) {
var ErrorReporter;
if (args == null) {
args = {};
}
ErrorReporter = require('../error-reporter');
return new ErrorReporter({
inSpecMode: args.test,
inDevMode: args.devMode
});
};
setupCoffeeScript = function() {
var CoffeeScript;
@ -120,7 +116,7 @@ parseCommandLine = function() {
process.exit(0);
}
if (args.version) {
process.stdout.write("" + version + "\n");
process.stdout.write(version + "\n");
process.exit(0);
}
executedFrom = args['executed-from'];
@ -191,3 +187,5 @@ parseCommandLine = function() {
};
start();
}).call(this);

View file

@ -24,9 +24,25 @@ var logpath = path.join(tmpPath, 'edgehill-' + logpid + '.log');
module.exports = ErrorReporter = (function() {
function ErrorReporter() {
function ErrorReporter(modes) {
var self = this;
this.inSpecMode = modes.inSpecMode
this.inDevMode = modes.inDevMode
if (!this.inSpecMode) {
this._setupSentry();
this._cleanOldLogFiles();
this._setupNewLogFile();
this._hookProcessOutputs();
this._catchUncaughtErrors();
}
console.debug = _.bind(this.consoleDebug, this);
}
ErrorReporter.prototype._setupSentry = function() {
// Initialize the Sentry connector
this.client = new raven.Client('https://7a32cb0189ff4595a55c98ffb7939c46:f791c3c402b343068bed056b8b504dd5@sentry.nylas.com/4');
this.client.on('error', function(e) {
@ -34,10 +50,12 @@ module.exports = ErrorReporter = (function() {
console.log(e.statusCode);
return console.log(e.response);
});
}
// If we're the browser process, remove log files that are more than
// two days old. These log files get pretty big because we're logging
// so verbosely.
ErrorReporter.prototype._cleanOldLogFiles = function() {
if (process.type === 'browser') {
fs.readdir(tmpPath, function(err, files) {
if (err) {
@ -60,6 +78,11 @@ module.exports = ErrorReporter = (function() {
});
});
}
}
ErrorReporter.prototype._setupNewLogFile = function() {
this.shipLogsQueued = false;
this.shipLogsTime = 0;
// Open a file write stream to log output from this process
console.log("Streaming log data to "+logpath);
@ -71,7 +94,10 @@ module.exports = ErrorReporter = (function() {
fd: null,
mode: 0666
});
}
ErrorReporter.prototype._hookProcessOutputs = function() {
var self = this;
// Override stdout and stderr to pipe their output to the file
// in addition to calling through to the existing implementation
function hook_process_output(channel, callback) {
@ -95,32 +121,13 @@ module.exports = ErrorReporter = (function() {
hook_process_output('stderr', function(string, encoding, fd) {
self.appendLog.apply(self, [string]);
});
this.shipLogsQueued = false;
this.shipLogsTime = 0;
// Create a new console.debug option, which takes `true` (print)
// or `false`, don't print in console as the first parameter.
// This makes it easy for developers to turn on and off
// "verbose console" mode.
console.debug = function() {
var args = [];
var showIt = arguments[0];
for (var ii = 1; ii < arguments.length; ii++) {
args.push(arguments[ii]);
}
if ((self.dev === true) && (showIt === true)) {
console.log.apply(this, args);
}
self.appendLog.apply(self, [args]);
};
ErrorReporter.prototype._catchUncaughtErrors = function() {
var self = this;
// Link to the appropriate error handlers for the browser
// or renderer process
if (process.type === 'renderer') {
this.spec = atom.inSpecMode();
this.dev = atom.inDevMode();
atom.onDidThrowError(function(_arg) {
return self.reportError(_arg.originalError, {
'message': _arg.message
@ -146,7 +153,25 @@ module.exports = ErrorReporter = (function() {
}
}
// Create a new console.debug option, which takes `true` (print)
// or `false`, don't print in console as the first parameter.
// This makes it easy for developers to turn on and off
// "verbose console" mode.
ErrorReporter.prototype.consoleDebug = function() {
var args = [];
var showIt = arguments[0];
for (var ii = 1; ii < arguments.length; ii++) {
args.push(arguments[ii]);
}
if ((this.dev === true) && (showIt === true)) {
console.log.apply(this, args);
}
this.appendLog.apply(this, [args]);
}
ErrorReporter.prototype.appendLog = function(obj) {
if (this.inSpecMode) { return };
try {
var message = JSON.stringify({
host: this.loghost,
@ -170,6 +195,8 @@ module.exports = ErrorReporter = (function() {
};
ErrorReporter.prototype.shipLogs = function(reason) {
if (this.inSpecMode) { return };
if (!this.shipLogsQueued) {
var timeSinceLogShip = Date.now() - this.shipLogsTime;
if (timeSinceLogShip > 20000) {
@ -186,6 +213,8 @@ module.exports = ErrorReporter = (function() {
};
ErrorReporter.prototype.runShipLogsTask = function(reason) {
if (this.inSpecMode) { return };
var self = this;
this.shipLogsTime = Date.now();
@ -216,12 +245,7 @@ module.exports = ErrorReporter = (function() {
};
ErrorReporter.prototype.reportError = function(err, metadata) {
if (this.spec) {
return;
}
if (this.dev) {
return;
}
if (this.inSpecMode || this.inDevMode) { return };
this.client.captureError(err, {
extra: metadata,

View file

@ -32,14 +32,17 @@ module.exports =
class PackageManager
EmitterMixin.includeInto(this)
constructor: ({configDirPath, @devMode, safeMode, @resourcePath}) ->
constructor: ({configDirPath, @devMode, safeMode, @resourcePath, @specMode}) ->
@emitter = new Emitter
@packageDirPaths = []
unless safeMode
if @specMode
@packageDirPaths.push(path.join(@resourcePath, "spec-nylas", "fixtures", "packages"))
else
@packageDirPaths.push(path.join(@resourcePath, "internal_packages"))
if not safeMode
if @devMode
@packageDirPaths.push(path.join(configDirPath, "dev", "packages"))
@packageDirPaths.push(path.join(configDirPath, "packages"))
@packageDirPaths.push(path.join(@resourcePath, "internal_packages"))
@loadedPackages = {}
@activePackages = {}