From c0e743bf0cad24bad590328925a32e9eee9810b2 Mon Sep 17 00:00:00 2001 From: Martin Hanzel Date: Wed, 5 Jun 2019 21:28:40 +0000 Subject: [PATCH] Migrate old notes app test from Karma to Jest --- app/assets/javascripts/notes.js | 4 + spec/frontend/helpers/jest_helpers.js | 24 + spec/frontend/helpers/timeout.js | 20 +- .../notes/old_notes_spec.js} | 495 +++++++++--------- 4 files changed, 292 insertions(+), 251 deletions(-) create mode 100644 spec/frontend/helpers/jest_helpers.js rename spec/{javascripts/notes_spec.js => frontend/notes/old_notes_spec.js} (68%) 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: `
  • @@ -177,35 +182,27 @@ describe('Notes', function() { note: sampleComment, valid: true, }; - $form = $('form.js-main-target-form'); + $notesContainer = $('ul.main-notes-list'); + const $form = $('form.js-main-target-form'); $form.find('textarea.js-note-text').val(sampleComment); - mock = new MockAdapter(axios); - mock.onPost(NOTES_POST_PATH).reply(200, noteEntity); + mockAxios.onPost(NOTES_POST_PATH).reply(200, noteEntity); }); - afterEach(() => { - mock.restore(); - }); - - it('updates note and resets edit form', done => { - spyOn(this.notes, 'revertNoteEditForm'); - spyOn(this.notes, 'setupNewNote'); + it('updates note and resets edit form', () => { + jest.spyOn(notes, 'revertNoteEditForm'); + jest.spyOn(notes, 'setupNewNote'); $('.js-comment-button').click(); - setTimeout(() => { - const $targetNote = $notesContainer.find(`#note_${noteEntity.id}`); - const updatedNote = Object.assign({}, noteEntity); - updatedNote.note = 'bar'; - this.notes.updateNote(updatedNote, $targetNote); + const $targetNote = $notesContainer.find(`#note_${noteEntity.id}`); + const updatedNote = Object.assign({}, noteEntity); + updatedNote.note = 'bar'; + notes.updateNote(updatedNote, $targetNote); - expect(this.notes.revertNoteEditForm).toHaveBeenCalledWith($targetNote); - expect(this.notes.setupNewNote).toHaveBeenCalled(); - - done(); - }); + expect(notes.revertNoteEditForm).toHaveBeenCalledWith($targetNote); + expect(notes.setupNewNote).toHaveBeenCalled(); }); }); @@ -215,32 +212,44 @@ describe('Notes', function() { beforeEach(() => { $note = $(`
    `); - spyOn($note, 'filter').and.callThrough(); - spyOn($note, 'toggleClass').and.callThrough(); + jest.spyOn($note, 'filter'); + jest.spyOn($note, 'toggleClass'); }); + afterEach(() => { + expect(typeof urlUtility.getLocationHash.mock).toBe('object'); + urlUtility.getLocationHash.mockRestore(); + expect(urlUtility.getLocationHash.mock).toBeUndefined(); + expect(urlUtility.getLocationHash()).toBeNull(); + }); + + // urlUtility is a dependency of the notes module. Its getLocatinHash() method should be called internally. + it('sets target when hash matches', () => { - spyOnDependency(Notes, 'getLocationHash').and.returnValue(hash); + jest.spyOn(urlUtility, 'getLocationHash').mockReturnValueOnce(hash); Notes.updateNoteTargetSelector($note); + expect(urlUtility.getLocationHash).toHaveBeenCalled(); expect($note.filter).toHaveBeenCalledWith(`#${hash}`); expect($note.toggleClass).toHaveBeenCalledWith('target', true); }); it('unsets target when hash does not match', () => { - spyOnDependency(Notes, 'getLocationHash').and.returnValue('note_doesnotexist'); + jest.spyOn(urlUtility, 'getLocationHash').mockReturnValueOnce('note_doesnotexist'); Notes.updateNoteTargetSelector($note); + expect(urlUtility.getLocationHash).toHaveBeenCalled(); expect($note.toggleClass).toHaveBeenCalledWith('target', false); }); it('unsets target when there is not a hash fragment anymore', () => { - spyOnDependency(Notes, 'getLocationHash').and.returnValue(null); + jest.spyOn(urlUtility, 'getLocationHash').mockReturnValueOnce(null); Notes.updateNoteTargetSelector($note); + expect(urlUtility.getLocationHash).toHaveBeenCalled(); expect($note.toggleClass).toHaveBeenCalledWith('target', false); }); }); @@ -257,28 +266,28 @@ describe('Notes', function() { note: 'heya', html: '
    heya
    ', }; - $notesList = jasmine.createSpyObj('$notesList', ['find', 'append']); + $notesList = createSpyObj('$notesList', ['find', 'append']); - notes = jasmine.createSpyObj('notes', [ + notes = createSpyObj('notes', [ 'setupNewNote', 'refresh', 'collapseLongCommitList', 'updateNotesCount', 'putConflictEditWarningInPlace', ]); - notes.taskList = jasmine.createSpyObj('tasklist', ['init']); + notes.taskList = createSpyObj('tasklist', ['init']); notes.note_ids = []; notes.updatedNotesTrackingMap = {}; - spyOn(Notes, 'isNewNote').and.callThrough(); - spyOn(Notes, 'isUpdatedNote').and.callThrough(); - spyOn(Notes, 'animateAppendNote').and.callThrough(); - spyOn(Notes, 'animateUpdateNote').and.callThrough(); + jest.spyOn(Notes, 'isNewNote'); + jest.spyOn(Notes, 'isUpdatedNote'); + jest.spyOn(Notes, 'animateAppendNote'); + jest.spyOn(Notes, 'animateUpdateNote'); }); describe('when adding note', () => { it('should call .animateAppendNote', () => { - Notes.isNewNote.and.returnValue(true); + Notes.isNewNote.mockReturnValueOnce(true); Notes.prototype.renderNote.call(notes, note, null, $notesList); expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.html, $notesList); @@ -287,12 +296,12 @@ describe('Notes', function() { describe('when note was edited', () => { it('should call .animateUpdateNote', () => { - Notes.isNewNote.and.returnValue(false); - Notes.isUpdatedNote.and.returnValue(true); + Notes.isNewNote.mockReturnValueOnce(false); + Notes.isUpdatedNote.mockReturnValueOnce(true); const $note = $('
    '); - $notesList.find.and.returnValue($note); + $notesList.find.mockReturnValueOnce($note); const $newNote = $(note.html); - Notes.animateUpdateNote.and.returnValue($newNote); + Notes.animateUpdateNote.mockReturnValueOnce($newNote); Notes.prototype.renderNote.call(notes, note, null, $notesList); @@ -302,26 +311,26 @@ describe('Notes', function() { describe('while editing', () => { it('should update textarea if nothing has been touched', () => { - Notes.isNewNote.and.returnValue(false); - Notes.isUpdatedNote.and.returnValue(true); + Notes.isNewNote.mockReturnValueOnce(false); + Notes.isUpdatedNote.mockReturnValueOnce(true); const $note = $(`
    initial
    `); - $notesList.find.and.returnValue($note); + $notesList.find.mockReturnValueOnce($note); Notes.prototype.renderNote.call(notes, note, null, $notesList); expect($note.find('.js-note-text').val()).toEqual(note.note); }); it('should call .putConflictEditWarningInPlace', () => { - Notes.isNewNote.and.returnValue(false); - Notes.isUpdatedNote.and.returnValue(true); + Notes.isNewNote.mockReturnValueOnce(false); + Notes.isUpdatedNote.mockReturnValueOnce(true); const $note = $(`
    initial
    `); - $notesList.find.and.returnValue($note); + $notesList.find.mockReturnValueOnce($note); Notes.prototype.renderNote.call(notes, note, null, $notesList); expect(notes.putConflictEditWarningInPlace).toHaveBeenCalledWith(note, $note); @@ -386,32 +395,32 @@ describe('Notes', function() { discussion_resolvable: false, diff_discussion_html: false, }; - $form = jasmine.createSpyObj('$form', ['closest', 'find']); + $form = createSpyObj('$form', ['closest', 'find']); $form.length = 1; - row = jasmine.createSpyObj('row', ['prevAll', 'first', 'find']); + row = createSpyObj('row', ['prevAll', 'first', 'find']); - notes = jasmine.createSpyObj('notes', ['isParallelView', 'updateNotesCount']); + notes = createSpyObj('notes', ['isParallelView', 'updateNotesCount']); notes.note_ids = []; - spyOn(Notes, 'isNewNote'); - spyOn(Notes, 'animateAppendNote'); - Notes.isNewNote.and.returnValue(true); - notes.isParallelView.and.returnValue(false); - row.prevAll.and.returnValue(row); - row.first.and.returnValue(row); - row.find.and.returnValue(row); + jest.spyOn(Notes, 'isNewNote'); + jest.spyOn(Notes, 'animateAppendNote').mockImplementation(); + Notes.isNewNote.mockReturnValue(true); + notes.isParallelView.mockReturnValue(false); + row.prevAll.mockReturnValue(row); + row.first.mockReturnValue(row); + row.find.mockReturnValue(row); }); describe('Discussion root note', () => { let body; beforeEach(() => { - body = jasmine.createSpyObj('body', ['attr']); + body = createSpyObj('body', ['attr']); discussionContainer = { length: 0 }; - $form.closest.and.returnValues(row, $form); - $form.find.and.returnValues(discussionContainer); - body.attr.and.returnValue(''); + $form.closest.mockReturnValueOnce(row).mockReturnValue($form); + $form.find.mockReturnValue(discussionContainer); + body.attr.mockReturnValue(''); }); it('should call Notes.animateAppendNote', () => { @@ -432,7 +441,9 @@ describe('Notes', function() { line.id = note.discussion_line_code; document.body.appendChild(line); - $form.closest.and.returnValues($form); + // Override mocks for this single test + $form.closest.mockReset(); + $form.closest.mockReturnValue($form); Notes.prototype.renderDiscussionNote.call(notes, note, $form); @@ -444,8 +455,8 @@ describe('Notes', function() { beforeEach(() => { discussionContainer = { length: 1 }; - $form.closest.and.returnValues(row, $form); - $form.find.and.returnValues(discussionContainer); + $form.closest.mockReturnValueOnce(row).mockReturnValueOnce($form); + $form.find.mockReturnValue(discussionContainer); Notes.prototype.renderDiscussionNote.call(notes, note, $form); }); @@ -463,7 +474,7 @@ describe('Notes', function() { beforeEach(() => { noteHTML = '
    '; - $notesList = jasmine.createSpyObj('$notesList', ['append']); + $notesList = createSpyObj('$notesList', ['append']); $resultantNote = Notes.animateAppendNote(noteHTML, $notesList); }); @@ -484,7 +495,7 @@ describe('Notes', function() { beforeEach(() => { noteHTML = '
    '; - $note = jasmine.createSpyObj('$note', ['replaceWith']); + $note = createSpyObj('$note', ['replaceWith']); $updatedNote = Notes.animateUpdateNote(noteHTML, $note); }); @@ -515,7 +526,6 @@ describe('Notes', function() { describe('postComment & updateComment', () => { const sampleComment = 'foo'; - const updatedComment = 'bar'; const note = { id: 1234, html: `
  • @@ -524,22 +534,20 @@ describe('Notes', function() { note: sampleComment, valid: true, }; + let notes; let $form; let $notesContainer; - let mock; function mockNotesPost() { - mock.onPost(NOTES_POST_PATH).reply(200, note); + mockAxios.onPost(NOTES_POST_PATH).reply(200, note); } function mockNotesPostError() { - mock.onPost(NOTES_POST_PATH).networkError(); + mockAxios.onPost(NOTES_POST_PATH).networkError(); } beforeEach(() => { - mock = new MockAdapter(axios); - - this.notes = new Notes('', []); + notes = new Notes('', []); window.gon.current_username = 'root'; window.gon.current_user_fullname = 'Administrator'; $form = $('form.js-main-target-form'); @@ -547,10 +555,6 @@ describe('Notes', function() { $form.find('textarea.js-note-text').val(sampleComment); }); - afterEach(() => { - mock.restore(); - }); - it('should show placeholder note while new comment is being posted', () => { mockNotesPost(); @@ -564,9 +568,8 @@ describe('Notes', function() { $('.js-comment-button').click(); - setTimeout(() => { + setImmediate(() => { expect($notesContainer.find('.note.being-posted').length).toEqual(0); - done(); }); }); @@ -580,12 +583,12 @@ describe('Notes', function() { preventDefault() {}, target: $submitButton, }; - mock.onPost(NOTES_POST_PATH).replyOnce(() => { + mockAxios.onPost(NOTES_POST_PATH).replyOnce(() => { expect($submitButton).toBeDisabled(); return [200, note]; }); - this.notes + notes .postComment(dummyEvent) .then(() => { expect($submitButton).not.toBeDisabled(); @@ -600,9 +603,8 @@ describe('Notes', function() { $('.js-comment-button').click(); - setTimeout(() => { + setImmediate(() => { expect($notesContainer.find(`#note_${note.id}`).length).toBeGreaterThan(0); - done(); }); }); @@ -612,48 +614,49 @@ describe('Notes', function() { $('.js-comment-button').click(); - setTimeout(() => { + setImmediate(() => { expect($form.find('textarea.js-note-text').val()).toEqual(''); - done(); }); }); it('should show flash error message when new comment failed to be posted', done => { mockNotesPostError(); + jest.spyOn(notes, 'addFlash'); $('.js-comment-button').click(); - setTimeout(() => { - expect( - $notesContainer - .parent() - .find('.flash-container .flash-text') - .is(':visible'), - ).toEqual(true); - + setImmediate(() => { + expect(notes.addFlash).toHaveBeenCalled(); + // JSDom doesn't support the :visible selector yet + expect(notes.flashContainer.style.display).not.toBe('none'); done(); }); }); + // This is a bad test carried over from the Karma -> Jest migration. + // The corresponding test in the Karma suite tests for + // elements and methods that don't actually exist, and gives a false + // positive pass. + /* it('should show flash error message when comment failed to be updated', done => { mockNotesPost(); + jest.spyOn(notes, 'addFlash').mockName('addFlash'); $('.js-comment-button').click(); - timeoutPromise() + deferredPromise() .then(() => { const $noteEl = $notesContainer.find(`#note_${note.id}`); $noteEl.find('.js-note-edit').click(); $noteEl.find('textarea.js-note-text').val(updatedComment); - mock.restore(); - mockNotesPostError(); $noteEl.find('.js-comment-save-button').click(); + notes.updateComment({preventDefault: () => {}}); }) - .then(timeoutPromise) + .then(() => deferredPromise()) .then(() => { const $updatedNoteEl = $notesContainer.find(`#note_${note.id}`); @@ -665,12 +668,13 @@ describe('Notes', function() { .trim(), ).toEqual(sampleComment); // See if comment reverted back to original - expect($('.flash-container').is(':visible')).toEqual(true); // Flash error message shown - + expect(notes.addFlash).toHaveBeenCalled(); + expect(notes.flashContainer.style.display).not.toBe('none'); done(); }) .catch(done.fail); - }, 2000); + }, 5000); + */ }); describe('postComment with Slash commands', () => { @@ -687,13 +691,11 @@ describe('Notes', function() { }; let $form; let $notesContainer; - let mock; beforeEach(() => { - mock = new MockAdapter(axios); - mock.onPost(NOTES_POST_PATH).reply(200, note); + mockAxios.onPost(NOTES_POST_PATH).reply(200, note); - this.notes = new Notes('', []); + new Notes('', []); window.gon.current_username = 'root'; window.gon.current_user_fullname = 'Administrator'; gl.awardsHandler = { @@ -710,17 +712,13 @@ describe('Notes', function() { $form.find('textarea.js-note-text').val(sampleComment); }); - afterEach(() => { - mock.restore(); - }); - it('should remove slash command placeholder when comment with slash commands is done posting', done => { - spyOn(gl.awardsHandler, 'addAwardToEmojiBar').and.callThrough(); + jest.spyOn(gl.awardsHandler, 'addAwardToEmojiBar'); $('.js-comment-button').click(); expect($notesContainer.find('.system-note.being-posted').length).toEqual(1); // Placeholder shown - setTimeout(() => { + setImmediate(() => { expect($notesContainer.find('.system-note.being-posted').length).toEqual(0); // Placeholder removed done(); }); @@ -740,13 +738,11 @@ describe('Notes', function() { }; let $form; let $notesContainer; - let mock; beforeEach(() => { - mock = new MockAdapter(axios); - mock.onPost(NOTES_POST_PATH).reply(200, note); + mockAxios.onPost(NOTES_POST_PATH).reply(200, note); - this.notes = new Notes('', []); + new Notes('', []); window.gon.current_username = 'root'; window.gon.current_user_fullname = 'Administrator'; $form = $('form.js-main-target-form'); @@ -754,14 +750,10 @@ describe('Notes', function() { $form.find('textarea.js-note-text').html(sampleComment); }); - afterEach(() => { - mock.restore(); - }); - it('should not render a script tag', done => { $('.js-comment-button').click(); - setTimeout(() => { + setImmediate(() => { const $noteEl = $notesContainer.find(`#note_${note.id}`); $noteEl.find('.js-note-edit').click(); $noteEl.find('textarea.js-note-text').html(updatedComment); @@ -786,9 +778,10 @@ describe('Notes', function() { describe('getFormData', () => { let $form; let sampleComment; + let notes; beforeEach(() => { - this.notes = new Notes('', []); + notes = new Notes('', []); $form = $('form'); sampleComment = 'foobar'; @@ -796,7 +789,7 @@ describe('Notes', function() { it('should return form metadata object from form reference', () => { $form.find('textarea.js-note-text').val(sampleComment); - const { formData, formContent, formAction } = this.notes.getFormData($form); + const { formData, formContent, formAction } = notes.getFormData($form); expect(formData.indexOf(sampleComment)).toBeGreaterThan(-1); expect(formContent).toEqual(sampleComment); @@ -804,12 +797,12 @@ describe('Notes', function() { }); it('should return form metadata with sanitized formContent from form reference', () => { - spyOn(_, 'escape').and.callFake(htmlEscape); + jest.spyOn(_, 'escape'); sampleComment = ''; $form.find('textarea.js-note-text').val(sampleComment); - const { formContent } = this.notes.getFormData($form); + const { formContent } = notes.getFormData($form); expect(_.escape).toHaveBeenCalledWith(sampleComment); expect(formContent).toEqual('<script>alert("Boom!");</script>'); @@ -817,27 +810,29 @@ describe('Notes', function() { }); describe('hasQuickActions', () => { + let notes; + beforeEach(() => { - this.notes = new Notes('', []); + notes = new Notes('', []); }); it('should return true when comment begins with a quick action', () => { const sampleComment = '/wip\n/milestone %1.0\n/merge\n/unassign Merging this'; - const hasQuickActions = this.notes.hasQuickActions(sampleComment); + const hasQuickActions = notes.hasQuickActions(sampleComment); expect(hasQuickActions).toBeTruthy(); }); it('should return false when comment does NOT begin with a quick action', () => { const sampleComment = 'Hey, /unassign Merging this'; - const hasQuickActions = this.notes.hasQuickActions(sampleComment); + const hasQuickActions = notes.hasQuickActions(sampleComment); expect(hasQuickActions).toBeFalsy(); }); it('should return false when comment does NOT have any quick actions', () => { const sampleComment = 'Looking good, Awesome!'; - const hasQuickActions = this.notes.hasQuickActions(sampleComment); + const hasQuickActions = notes.hasQuickActions(sampleComment); expect(hasQuickActions).toBeFalsy(); }); @@ -845,25 +840,25 @@ describe('Notes', function() { describe('stripQuickActions', () => { it('should strip quick actions from the comment which begins with a quick action', () => { - this.notes = new Notes(); + const notes = new Notes(); const sampleComment = '/wip\n/milestone %1.0\n/merge\n/unassign Merging this'; - const stripedComment = this.notes.stripQuickActions(sampleComment); + const stripedComment = notes.stripQuickActions(sampleComment); expect(stripedComment).toBe(''); }); it('should strip quick actions from the comment but leaves plain comment if it is present', () => { - this.notes = new Notes(); + const notes = new Notes(); const sampleComment = '/wip\n/milestone %1.0\n/merge\n/unassign\nMerging this'; - const stripedComment = this.notes.stripQuickActions(sampleComment); + const stripedComment = notes.stripQuickActions(sampleComment); expect(stripedComment).toBe('Merging this'); }); it('should NOT strip string that has slashes within', () => { - this.notes = new Notes(); + const notes = new Notes(); const sampleComment = 'http://127.0.0.1:3000/root/gitlab-shell/issues/1'; - const stripedComment = this.notes.stripQuickActions(sampleComment); + const stripedComment = notes.stripQuickActions(sampleComment); expect(stripedComment).toBe(sampleComment); }); @@ -875,15 +870,16 @@ describe('Notes', function() { { name: 'title', description: 'Change title', params: [{}] }, { name: 'estimate', description: 'Set time estimate', params: [{}] }, ]; + let notes; beforeEach(() => { - this.notes = new Notes(); + notes = new Notes(); }); it('should return executing quick action description when note has single quick action', () => { const sampleComment = '/close'; - expect(this.notes.getQuickActionDescription(sampleComment, availableQuickActions)).toBe( + expect(notes.getQuickActionDescription(sampleComment, availableQuickActions)).toBe( 'Applying command to close this issue', ); }); @@ -891,7 +887,7 @@ describe('Notes', function() { it('should return generic multiple quick action description when note has multiple quick actions', () => { const sampleComment = '/close\n/title [Duplicate] Issue foobar'; - expect(this.notes.getQuickActionDescription(sampleComment, availableQuickActions)).toBe( + expect(notes.getQuickActionDescription(sampleComment, availableQuickActions)).toBe( 'Applying multiple commands', ); }); @@ -899,7 +895,7 @@ describe('Notes', function() { it('should return generic quick action description when available quick actions list is not populated', () => { const sampleComment = '/close\n/title [Duplicate] Issue foobar'; - expect(this.notes.getQuickActionDescription(sampleComment)).toBe('Applying command'); + expect(notes.getQuickActionDescription(sampleComment)).toBe('Applying command'); }); }); @@ -909,13 +905,14 @@ describe('Notes', function() { const currentUsername = 'root'; const currentUserFullname = 'Administrator'; const currentUserAvatar = 'avatar_url'; + let notes; beforeEach(() => { - this.notes = new Notes('', []); + notes = new Notes('', []); }); it('should return constructed placeholder element for regular note based on form contents', () => { - const $tempNote = this.notes.createPlaceholderNote({ + const $tempNote = notes.createPlaceholderNote({ formContent: sampleComment, uniqueId, isDiscussionNote: false, @@ -929,8 +926,8 @@ describe('Notes', function() { expect($tempNote.attr('id')).toEqual(uniqueId); expect($tempNote.hasClass('being-posted')).toBeTruthy(); expect($tempNote.hasClass('fade-in-half')).toBeTruthy(); - $tempNote.find('.timeline-icon > a, .note-header-info > a').each(function() { - expect($(this).attr('href')).toEqual(`/${currentUsername}`); + $tempNote.find('.timeline-icon > a, .note-header-info > a').each((i, el) => { + expect(el.getAttribute('href')).toEqual(`/${currentUsername}`); }); expect($tempNote.find('.timeline-icon .avatar').attr('src')).toEqual(currentUserAvatar); @@ -958,7 +955,7 @@ describe('Notes', function() { }); it('should return constructed placeholder element for discussion note based on form contents', () => { - const $tempNote = this.notes.createPlaceholderNote({ + const $tempNote = notes.createPlaceholderNote({ formContent: sampleComment, uniqueId, isDiscussionNote: true, @@ -972,7 +969,7 @@ describe('Notes', function() { it('should return a escaped user name', () => { const currentUserFullnameXSS = 'Foo '; - const $tempNote = this.notes.createPlaceholderNote({ + const $tempNote = notes.createPlaceholderNote({ formContent: sampleComment, uniqueId, isDiscussionNote: false, @@ -994,14 +991,15 @@ describe('Notes', function() { describe('createPlaceholderSystemNote', () => { const sampleCommandDescription = 'Applying command to close this issue'; const uniqueId = 'b1234-a4567'; + let notes; beforeEach(() => { - this.notes = new Notes('', []); - spyOn(_, 'escape').and.callFake(htmlEscape); + notes = new Notes('', []); + jest.spyOn(_, 'escape'); }); it('should return constructed placeholder element for system note based on form contents', () => { - const $tempNote = this.notes.createPlaceholderSystemNote({ + const $tempNote = notes.createPlaceholderSystemNote({ formContent: sampleCommandDescription, uniqueId, }); @@ -1020,29 +1018,28 @@ describe('Notes', function() { }); describe('appendFlash', () => { - beforeEach(() => { - this.notes = new Notes(); - }); - it('shows a flash message', () => { - this.notes.addFlash('Error message', FLASH_TYPE_ALERT, this.notes.parentTimeline.get(0)); + const notes = new Notes('', []); + notes.addFlash('Error message', FLASH_TYPE_ALERT, notes.parentTimeline.get(0)); - expect($('.flash-alert').is(':visible')).toBeTruthy(); + const flash = $('.flash-alert')[0]; + expect(document.contains(flash)).toBe(true); + expect(flash.parentNode.style.display).toBe('block'); }); }); describe('clearFlash', () => { beforeEach(() => { $(document).off('ajax:success'); - this.notes = new Notes(); }); it('hides visible flash message', () => { - this.notes.addFlash('Error message 1', FLASH_TYPE_ALERT, this.notes.parentTimeline.get(0)); - - this.notes.clearFlash(); - - expect($('.flash-alert').is(':visible')).toBeFalsy(); + const notes = new Notes('', []); + notes.addFlash('Error message 1', FLASH_TYPE_ALERT, notes.parentTimeline.get(0)); + const flash = $('.flash-alert')[0]; + notes.clearFlash(); + expect(flash.parentNode.style.display).toBe('none'); + expect(notes.flashContainer).toBeNull(); }); }); });