diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index d03f4508fb8..1c9ca180100 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -7,6 +7,10 @@ no-unused-vars, no-shadow, no-useless-escape, class-methods-use-this */ /* global ResolveService */ /* global mrRefreshWidgetUrl */ +/* +old_notes_spec.js is the spec for the legacy, jQuery notes application. It has nothing to do with the new, fancy Vue notes app. + */ + import $ from 'jquery'; import _ from 'underscore'; import Cookies from 'js-cookie'; diff --git a/spec/frontend/helpers/jest_helpers.js b/spec/frontend/helpers/jest_helpers.js new file mode 100644 index 00000000000..4a150be9935 --- /dev/null +++ b/spec/frontend/helpers/jest_helpers.js @@ -0,0 +1,24 @@ +/* eslint-disable import/prefer-default-export */ + +/* +@module + +This method provides convenience functions to help migrating from Karma/Jasmine to Jest. + +Try not to use these in new tests - this module is provided primarily for convenience of migrating tests. + */ + +/** + * Creates a plain JS object pre-populated with Jest spy functions. Useful for making simple mocks classes. + * + * @see https://jasmine.github.io/2.0/introduction.html#section-Spies:_%3Ccode%3EcreateSpyObj%3C/code%3E + * @param {string} baseName Human-readable name of the object. This is used for reporting purposes. + * @param methods {string[]} List of method names that will be added to the spy object. + */ +export function createSpyObj(baseName, methods) { + const obj = {}; + methods.forEach(method => { + obj[method] = jest.fn().mockName(`${baseName}#${method}`); + }); + return obj; +} diff --git a/spec/frontend/helpers/timeout.js b/spec/frontend/helpers/timeout.js index b30b7f1ce1e..e74598ae20a 100644 --- a/spec/frontend/helpers/timeout.js +++ b/spec/frontend/helpers/timeout.js @@ -1,5 +1,6 @@ const NS_PER_SEC = 1e9; const NS_PER_MS = 1e6; +const IS_DEBUGGING = process.execArgv.join(' ').includes('--inspect-brk'); let testTimeoutNS; @@ -8,6 +9,13 @@ export const setTestTimeout = newTimeoutMS => { jest.setTimeout(newTimeoutMS); }; +// Allows slow tests to set their own timeout. +// Useful for tests with jQuery, which is very slow in big DOMs. +let temporaryTimeoutNS = null; +export const setTestTimeoutOnce = newTimeoutMS => { + temporaryTimeoutNS = newTimeoutMS * NS_PER_MS; +}; + export const initializeTestTimeout = defaultTimeoutMS => { setTestTimeout(defaultTimeoutMS); @@ -19,12 +27,20 @@ export const initializeTestTimeout = defaultTimeoutMS => { }); afterEach(() => { + let timeoutNS = testTimeoutNS; + if (Number.isFinite(temporaryTimeoutNS)) { + timeoutNS = temporaryTimeoutNS; + temporaryTimeoutNS = null; + } + const [seconds, remainingNs] = process.hrtime(testStartTime); const elapsedNS = seconds * NS_PER_SEC + remainingNs; - if (elapsedNS > testTimeoutNS) { + // Disable the timeout error when debugging. It is meaningless because + // debugging always takes longer than the test timeout. + if (elapsedNS > timeoutNS && !IS_DEBUGGING) { throw new Error( - `Test took too long (${elapsedNS / NS_PER_MS}ms > ${testTimeoutNS / NS_PER_MS}ms)!`, + `Test took too long (${elapsedNS / NS_PER_MS}ms > ${timeoutNS / NS_PER_MS}ms)!`, ); } }); diff --git a/spec/javascripts/notes_spec.js b/spec/frontend/notes/old_notes_spec.js similarity index 68% rename from spec/javascripts/notes_spec.js rename to spec/frontend/notes/old_notes_spec.js index 394e3343be6..b57041cf4d1 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/frontend/notes/old_notes_spec.js @@ -1,84 +1,89 @@ -/* eslint-disable no-unused-expressions, no-var, object-shorthand */ +/* eslint-disable import/no-commonjs, no-new */ + import $ from 'jquery'; import _ from 'underscore'; import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; -import 'autosize'; -import '~/gl_form'; -import '~/lib/utils/text_utility'; +import * as urlUtility from '~/lib/utils/url_utility'; import '~/behaviors/markdown/render_gfm'; -import Notes from '~/notes'; -import timeoutPromise from './helpers/set_timeout_promise_helper'; +import { createSpyObj } from 'helpers/jest_helpers'; +import { setTestTimeoutOnce } from 'helpers/timeout'; +import { TEST_HOST } from 'helpers/test_constants'; -window.gon || (window.gon = {}); +// These must be imported synchronously because they pull dependencies +// from the DOM. +window.jQuery = $; +require('autosize'); +require('~/commons'); +require('~/notes'); + +const { Notes } = window; +const FLASH_TYPE_ALERT = 'alert'; +const NOTES_POST_PATH = /(.*)\/notes\?html=true$/; +const fixture = 'snippets/show.html'; +let mockAxios; + +window.project_uploads_path = `${TEST_HOST}/uploads`; +window.gon = window.gon || {}; window.gl = window.gl || {}; gl.utils = gl.utils || {}; +gl.utils.disableButtonIfEmptyField = () => {}; -const htmlEscape = comment => { - const escapedString = comment.replace(/["&'<>]/g, a => { - const escapedToken = { - '&': '&', - '<': '<', - '>': '>', - '"': '"', - "'": ''', - '`': '`', - }[a]; - - return escapedToken; - }); - - return escapedString; -}; - -describe('Notes', function() { - const FLASH_TYPE_ALERT = 'alert'; - const NOTES_POST_PATH = /(.*)\/notes\?html=true$/; - var fixture = 'snippets/show.html'; - preloadFixtures(fixture); - - beforeEach(function() { +describe('Old Notes (~/notes.js)', () => { + beforeEach(() => { + jest.useFakeTimers(); loadFixtures(fixture); - gl.utils.disableButtonIfEmptyField = _.noop; - window.project_uploads_path = 'http://test.host/uploads'; - $('body').attr('data-page', 'projects:merge_requets:show'); + + // Re-declare this here so that test_setup.js#beforeEach() doesn't + // overwrite it. + mockAxios = new MockAdapter(axios); + + $.ajax = () => { + throw new Error('$.ajax should not be called through!'); + }; + + // These jQuery+DOM tests are super flaky so increase the timeout to avoid + // random failures. + // It seems that running tests in parallel increases failure rate. + jest.setTimeout(4000); + setTestTimeoutOnce(4000); }); - afterEach(() => { - // Undo what we did to the shared
- $('body').removeAttr('data-page'); + afterEach(done => { + // The Notes component sets a polling interval. Clear it after every run. + // Make sure to use jest.runOnlyPendingTimers() instead of runAllTimers(). + jest.clearAllTimers(); + + setImmediate(() => { + // Wait for any requests to resolve, otherwise we get failures about + // unmocked requests. + mockAxios.restore(); + done(); + }); + }); + + it('loads the Notes class into the DOM', () => { + expect(Notes).toBeDefined(); + expect(Notes.name).toBe('Notes'); }); describe('addBinding', () => { it('calls postComment when comment button is clicked', () => { - spyOn(Notes.prototype, 'postComment'); - this.notes = new Notes('', []); + jest.spyOn(Notes.prototype, 'postComment'); + new window.Notes('', []); $('.js-comment-button').click(); - expect(Notes.prototype.postComment).toHaveBeenCalled(); }); }); - describe('task lists', function() { - let mock; - - beforeEach(function() { - spyOn(axios, 'patch').and.callFake(() => new Promise(() => {})); - mock = new MockAdapter(axios); - mock.onAny().reply(200, {}); - - $('.js-comment-button').on('click', function(e) { - e.preventDefault(); - }); - this.notes = new Notes('', []); + describe('task lists', () => { + beforeEach(() => { + mockAxios.onAny().reply(200, {}); + new Notes('', []); }); - afterEach(() => { - mock.restore(); - }); - - it('modifies the Markdown field', function() { + it('modifies the Markdown field', () => { const changeEvent = document.createEvent('HTMLEvents'); changeEvent.initEvent('change', true, true); $('input[type=checkbox]') @@ -88,7 +93,9 @@ describe('Notes', function() { expect($('.js-task-list-field.original-task-list').val()).toBe('- [x] Task List Item'); }); - it('submits an ajax request on tasklist:changed', function(done) { + it('submits an ajax request on tasklist:changed', () => { + jest.spyOn(axios, 'patch'); + const lineNumber = 8; const lineSource = '- [ ] item 8'; const index = 3; @@ -99,76 +106,74 @@ describe('Notes', function() { detail: { lineNumber, lineSource, index, checked }, }); - setTimeout(() => { - expect(axios.patch).toHaveBeenCalledWith(undefined, { - note: { - note: '', - lock_version: undefined, - update_task: { index, checked, line_number: lineNumber, line_source: lineSource }, - }, - }); - - done(); + expect(axios.patch).toHaveBeenCalledWith(undefined, { + note: { + note: '', + lock_version: undefined, + update_task: { index, checked, line_number: lineNumber, line_source: lineSource }, + }, }); }); }); - describe('comments', function() { - var textarea = '.js-note-text'; + describe('comments', () => { + let notes; + let autosizeSpy; + let textarea; - beforeEach(function() { - this.notes = new Notes('', []); + beforeEach(() => { + notes = new Notes('', []); - this.autoSizeSpy = spyOnEvent($(textarea), 'autosize:update'); - spyOn(this.notes, 'renderNote').and.stub(); - - $(textarea).data('autosave', { - reset: function() {}, + textarea = $('.js-note-text'); + textarea.data('autosave', { + reset: () => {}, }); + autosizeSpy = jest.fn(); + $(textarea).on('autosize:update', autosizeSpy); + + jest.spyOn(notes, 'renderNote'); $('.js-comment-button').on('click', e => { const $form = $(this); e.preventDefault(); - this.notes.addNote($form); - this.notes.reenableTargetFormSubmitButton(e); - this.notes.resetMainTargetForm(e); + notes.addNote($form, {}); + notes.reenableTargetFormSubmitButton(e); + notes.resetMainTargetForm(e); }); }); - it('autosizes after comment submission', function() { - $(textarea).text('This is an example comment note'); - - expect(this.autoSizeSpy).not.toHaveBeenTriggered(); - + it('autosizes after comment submission', () => { + textarea.text('This is an example comment note'); + expect(autosizeSpy).not.toHaveBeenCalled(); $('.js-comment-button').click(); - - expect(this.autoSizeSpy).toHaveBeenTriggered(); + expect(autosizeSpy).toHaveBeenCalled(); }); - it('should not place escaped text in the comment box in case of error', function() { + it('should not place escaped text in the comment box in case of error', () => { const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); + jest.spyOn($, 'ajax').mockReturnValueOnce(deferred); $(textarea).text('A comment with `markup`.'); deferred.reject(); $('.js-comment-button').click(); - expect($(textarea).val()).toEqual('A comment with `markup`.'); + expect($(textarea).val()).toBe('A comment with `markup`.'); + + $.ajax.mockRestore(); + expect($.ajax.mock).toBeUndefined(); }); }); describe('updateNote', () => { - let sampleComment; + let notes; let noteEntity; - let $form; let $notesContainer; - let mock; beforeEach(() => { - this.notes = new Notes('', []); + notes = new Notes('', []); window.gon.current_username = 'root'; window.gon.current_user_fullname = 'Administrator'; - sampleComment = 'foo'; + const sampleComment = 'foo'; noteEntity = { id: 1234, html: `