From 13b60eb75b99abb23f41c0d899d6e40eefa641cc Mon Sep 17 00:00:00 2001 From: "Luke \"Jared\" Bennett" Date: Thu, 13 Apr 2017 17:17:41 +0100 Subject: [PATCH] [ci skip] Index and singleton improvements with some more unit, more to come --- app/assets/javascripts/raven/index.js | 6 ++- app/assets/javascripts/raven/raven_config.js | 25 +++++----- spec/helpers/sentry_helper_spec.rb | 15 ++++-- spec/javascripts/raven/index_spec.js | 42 ++++++++++++++--- spec/javascripts/raven/raven_config_spec.js | 13 +----- spec/javascripts/spec_helper.js | 49 -------------------- 6 files changed, 66 insertions(+), 84 deletions(-) delete mode 100644 spec/javascripts/spec_helper.js diff --git a/app/assets/javascripts/raven/index.js b/app/assets/javascripts/raven/index.js index 6cc81248e6b..373f9f29c79 100644 --- a/app/assets/javascripts/raven/index.js +++ b/app/assets/javascripts/raven/index.js @@ -1,10 +1,12 @@ import RavenConfig from './raven_config'; -RavenConfig.init({ +const index = RavenConfig.init.bind(RavenConfig, { sentryDsn: gon.sentry_dsn, currentUserId: gon.current_user_id, whitelistUrls: [gon.gitlab_url], isProduction: gon.is_production, }); -export default RavenConfig; +index(); + +export default index; diff --git a/app/assets/javascripts/raven/raven_config.js b/app/assets/javascripts/raven/raven_config.js index 5510dd2752d..1157a10d96f 100644 --- a/app/assets/javascripts/raven/raven_config.js +++ b/app/assets/javascripts/raven/raven_config.js @@ -1,32 +1,35 @@ import Raven from 'raven-js'; +import $ from 'jquery'; -class RavenConfig { - static init(options = {}) { +const RavenConfig = { + init(options = {}) { this.options = options; this.configure(); this.bindRavenErrors(); if (this.options.currentUserId) this.setUser(); - } - static configure() { + return this; + }, + + configure() { Raven.config(this.options.sentryDsn, { whitelistUrls: this.options.whitelistUrls, environment: this.options.isProduction ? 'production' : 'development', }).install(); - } + }, - static setUser() { + setUser() { Raven.setUserContext({ id: this.options.currentUserId, }); - } + }, - static bindRavenErrors() { + bindRavenErrors() { $(document).on('ajaxError.raven', this.handleRavenErrors); - } + }, - static handleRavenErrors(event, req, config, err) { + handleRavenErrors(event, req, config, err) { const error = err || req.statusText; Raven.captureMessage(error, { @@ -40,7 +43,7 @@ class RavenConfig { event, }, }); - } + }, } export default RavenConfig; diff --git a/spec/helpers/sentry_helper_spec.rb b/spec/helpers/sentry_helper_spec.rb index 35ecf9355e1..ff218235cd1 100644 --- a/spec/helpers/sentry_helper_spec.rb +++ b/spec/helpers/sentry_helper_spec.rb @@ -3,13 +3,20 @@ require 'spec_helper' describe SentryHelper do describe '#sentry_dsn_public' do it 'returns nil if no sentry_dsn is set' do - allow(ApplicationSetting.current).to receive(:sentry_dsn).and_return(nil) - expect(helper.sentry_dsn_public).to eq(nil) + mock_sentry_dsn(nil) + + expect(helper.sentry_dsn_public).to eq nil end it 'returns the uri string with no password if sentry_dsn is set' do - allow(ApplicationSetting.current).to receive(:sentry_dsn).and_return('https://test:dsn@host/path') - expect(helper.sentry_dsn_public).to eq('https://test@host/path') + mock_sentry_dsn('https://test:dsn@host/path') + + expect(helper.sentry_dsn_public).to eq 'https://test@host/path' end end + + def mock_sentry_dsn(value) + allow_message_expectations_on_nil + allow(ApplicationSetting.current).to receive(:sentry_dsn).and_return(value) + end end diff --git a/spec/javascripts/raven/index_spec.js b/spec/javascripts/raven/index_spec.js index 51e84a6dbee..efde37b1f8e 100644 --- a/spec/javascripts/raven/index_spec.js +++ b/spec/javascripts/raven/index_spec.js @@ -1,11 +1,41 @@ -import RavenConfig from '~/raven/index'; +import RavenConfig from '~/raven/raven_config'; +import index from '~/raven/index'; -describe('RavenConfig options', () => { - it('should set sentryDsn'); +fdescribe('RavenConfig options', () => { + let sentryDsn; + let currentUserId; + let gitlabUrl; + let isProduction; + let indexReturnValue; - it('should set currentUserId'); + beforeEach(() => { + sentryDsn = 'sentryDsn'; + currentUserId = 'currentUserId'; + gitlabUrl = 'gitlabUrl'; + isProduction = 'isProduction'; - it('should set whitelistUrls'); + window.gon = { + sentry_dsn: sentryDsn, + current_user_id: currentUserId, + gitlab_url: gitlabUrl, + is_production: isProduction, + }; - it('should set isProduction'); + spyOn(RavenConfig.init, 'bind'); + + indexReturnValue = index(); + }); + + it('should init with .sentryDsn, .currentUserId, .whitelistUrls and .isProduction', () => { + expect(RavenConfig.init.bind).toHaveBeenCalledWith(RavenConfig, { + sentryDsn, + currentUserId, + whitelistUrls: [gitlabUrl], + isProduction, + }); + }); + + it('should return RavenConfig', () => { + expect(indexReturnValue).toBe(RavenConfig); + }); }); diff --git a/spec/javascripts/raven/raven_config_spec.js b/spec/javascripts/raven/raven_config_spec.js index 2b63f56d719..64cf63702bc 100644 --- a/spec/javascripts/raven/raven_config_spec.js +++ b/spec/javascripts/raven/raven_config_spec.js @@ -1,8 +1,7 @@ import Raven from 'raven-js'; import RavenConfig from '~/raven/raven_config'; -import ClassSpecHelper from '../helpers/class_spec_helper'; -fdescribe('RavenConfig', () => { +describe('RavenConfig', () => { describe('init', () => { beforeEach(() => { spyOn(RavenConfig, 'configure'); @@ -10,8 +9,6 @@ fdescribe('RavenConfig', () => { spyOn(RavenConfig, 'setUser'); }); - ClassSpecHelper.itShouldBeAStaticMethod(RavenConfig, 'init'); - describe('when called', () => { let options; @@ -58,8 +55,6 @@ fdescribe('RavenConfig', () => { }); describe('configure', () => { - ClassSpecHelper.itShouldBeAStaticMethod(RavenConfig, 'configure'); - describe('when called', () => { let options; let raven; @@ -112,24 +107,18 @@ fdescribe('RavenConfig', () => { }); describe('setUser', () => { - ClassSpecHelper.itShouldBeAStaticMethod(RavenConfig, 'setUser'); - describe('when called', () => { beforeEach(() => {}); }); }); describe('bindRavenErrors', () => { - ClassSpecHelper.itShouldBeAStaticMethod(RavenConfig, 'bindRavenErrors'); - describe('when called', () => { beforeEach(() => {}); }); }); describe('handleRavenErrors', () => { - ClassSpecHelper.itShouldBeAStaticMethod(RavenConfig, 'handleRavenErrors'); - describe('when called', () => { beforeEach(() => {}); }); diff --git a/spec/javascripts/spec_helper.js b/spec/javascripts/spec_helper.js deleted file mode 100644 index ee6e6279ac9..00000000000 --- a/spec/javascripts/spec_helper.js +++ /dev/null @@ -1,49 +0,0 @@ -/* eslint-disable space-before-function-paren */ -// PhantomJS (Teaspoons default driver) doesn't have support for -// Function.prototype.bind, which has caused confusion. Use this polyfill to -// avoid the confusion. -/*= require support/bind-poly */ - -// You can require your own javascript files here. By default this will include -// everything in application, however you may get better load performance if you -// require the specific files that are being used in the spec that tests them. -/*= require jquery */ -/*= require jquery.turbolinks */ -/*= require bootstrap */ -/*= require underscore */ -/*= require es6-promise.auto */ - -// Teaspoon includes some support files, but you can use anything from your own -// support path too. -// require support/jasmine-jquery-1.7.0 -// require support/jasmine-jquery-2.0.0 -/*= require support/jasmine-jquery-2.1.0 */ - -// require support/sinon -// require support/your-support-file -// Deferring execution -// If you're using CommonJS, RequireJS or some other asynchronous library you can -// defer execution. Call Teaspoon.execute() after everything has been loaded. -// Simple example of a timeout: -// Teaspoon.defer = true -// setTimeout(Teaspoon.execute, 1000) -// Matching files -// By default Teaspoon will look for files that match -// _spec.{js,js.es6}. Add a filename_spec.js file in your spec path -// and it'll be included in the default suite automatically. If you want to -// customize suites, check out the configuration in teaspoon_env.rb -// Manifest -// If you'd rather require your spec files manually (to control order for -// instance) you can disable the suite matcher in the configuration and use this -// file as a manifest. -// For more information: http://github.com/modeset/teaspoon - -// set our fixtures path -jasmine.getFixtures().fixturesPath = '/teaspoon/fixtures'; -jasmine.getJSONFixtures().fixturesPath = '/teaspoon/fixtures'; - -// defined in ActionDispatch::TestRequest -// see https://github.com/rails/rails/blob/v4.2.7.1/actionpack/lib/action_dispatch/testing/test_request.rb#L7 -window.gl = window.gl || {}; -window.gl.TEST_HOST = 'http://test.host'; -window.gon = window.gon || {};