Merge branch 'clean-up-issue_spec.js' into 'master'

Replace static fixture by generated one in issue_spec.js

## What does this MR do?

- clean up `issue_spec.js`
- introduce an alternative approach to #19445
  - rename `rake teaspoon` to `rake teaspoon:tests`
  - introduce `rake teaspoon:fixtures` which generates fixtures using RSpec
  - introduce `rake teaspoon` which runs `rake teaspoon:fixtures` and `rake teaspoon:tests`

## Why was this MR needed?

- many duplications
- missing existence checks
- missing conditions
- static fixtures don't match real views

## Reasoning

I want to explain some of my decisions here, so that they stay visible for future discussions.

### Why not HAML?

- same number of HAML templates as number of fixtures (many input files)
- embedded logic less readable
- can not be rendered by JavaScript (because of inline Ruby)

### Why RSpec?

- real controllers for fixtures
- spys available for mocking
- easily report failed fixture generations

### Why not magic_lamp? (#19445)

- introduces another dependency/tool
- needs to run a server concurrently to teaspoon
- makes it harder to use a JavaScript test runner
- static HTML files serve faster

See merge request !6059

.gitlab-ci.yml

@@ -249,7 +249,7 @@ teaspoon:
     - curl --silent --location https://deb.nodesource.com/setup_6.x | bash -
     - apt-get install --assume-yes nodejs
     - npm install --global istanbul
-    - teaspoon
+    - rake teaspoon
   artifacts:
     name: coverage-javascript
     expire_in: 31d

CHANGELOG.md

@@ -157,6 +157,7 @@ Please view this file on the master branch, on stable branches it's out of date.
   - Only update issuable labels if they have been changed
   - Take filters in account in issuable counters. !6496
   - Use custom Ruby images to test builds (registry.dev.gitlab.org/gitlab/gitlab-build-images:*)
+  - Replace static issue fixtures by script !6059 (winniehell)
   - Append issue template to existing description !6149 (Joseph Frazier)
   - Trending projects now only show public projects and the list of projects is cached for a day
   - Memoize Gitlab Shell's secret token (!6599, Justin DiPierro)

doc/development/frontend.md

@@ -185,6 +185,20 @@ again in the future.
 See [the Testing Standards and Style Guidelines](testing.md) for more
 information.
 
+### Running frontend tests
+
+`rake teaspoon` runs the frontend-only (JavaScript) tests.
+It consists of two subtasks:
+
+- `rake teaspoon:fixtures` (re-)generates fixtures
+- `rake teaspoon:tests` actually executes the tests
+
+As long as the fixtures don't change, `rake teaspoon:tests` is sufficient
+(and saves you some time).
+
+Please note: Not all of the frontend fixtures are generated. Some are still static
+files. These will not be touched by `rake teaspoon:fixtures`.
+
 ## Supported browsers
 
 For our currently-supported browsers, see our [requirements][requirements].

lib/tasks/teaspoon.rake

@@ -0,0 +1,23 @@
+Rake::Task['teaspoon'].clear if Rake::Task.task_defined?('teaspoon')
+
+namespace :teaspoon do
+  desc 'GitLab | Teaspoon | Generate fixtures for JavaScript tests'
+  RSpec::Core::RakeTask.new(:fixtures) do |t|
+    ENV['NO_KNAPSACK'] = 'true'
+    t.pattern = 'spec/javascripts/fixtures/*.rb'
+    t.rspec_opts = '--format documentation'
+  end
+
+  desc 'GitLab | Teaspoon | Run JavaScript tests'
+  task :tests do
+    require "teaspoon/console"
+    options = {}
+    abort('rake teaspoon:tests failed') if Teaspoon::Console.new(options).failures?
+  end
+end
+
+desc 'GitLab | Teaspoon | Shortcut for teaspoon:fixtures and teaspoon:tests'
+task :teaspoon do
+  Rake::Task['teaspoon:fixtures'].invoke
+  Rake::Task['teaspoon:tests'].invoke
+end

spec/javascripts/fixtures/.gitignore

@@ -0,0 +1 @@
+*.html.raw

spec/javascripts/fixtures/issues.rb

@@ -0,0 +1,44 @@
+require 'spec_helper'
+
+describe Projects::IssuesController, '(JavaScript fixtures)', type: :controller do
+  include JavaScriptFixturesHelpers
+
+  let(:admin) { create(:admin) }
+  let(:project) { create(:project_empty_repo) }
+
+  render_views
+
+  before(:all) do
+    clean_frontend_fixtures('issues/')
+  end
+
+  before(:each) do
+    sign_in(admin)
+  end
+
+  it 'issues/open-issue.html.raw' do |example|
+    render_issue(example.description, create(:issue, project: project))
+  end
+
+  it 'issues/closed-issue.html.raw' do |example|
+    render_issue(example.description, create(:closed_issue, project: project))
+  end
+
+  it 'issues/issue-with-task-list.html.raw' do |example|
+    issue = create(:issue, project: project)
+    issue.update(description: '- [ ] Task List Item')
+    render_issue(example.description, issue)
+  end
+
+  private
+
+  def render_issue(fixture_file_name, issue)
+    get :show,
+      namespace_id: project.namespace.to_param,
+      project_id: project.to_param,
+      id: issue.to_param
+
+    expect(response).to be_success
+    store_frontend_fixture(response, fixture_file_name)
+  end
+end

spec/javascripts/fixtures/issues_show.html.haml

@@ -1,23 +0,0 @@
-:css
-  .hidden { display: none !important; }
-
-.flash-container.flash-container-page
-  .flash-alert
-  .flash-notice
-
-.status-box.status-box-open Open
-.status-box.status-box-closed.hidden Closed
-%a.btn-close{"href" => "http://gitlab.com/issues/6/close"} Close
-%a.btn-reopen.hidden{"href" => "http://gitlab.com/issues/6/reopen"} Reopen
-
-.detail-page-description
-  .description.js-task-list-container
-    .wiki
-      %ul.task-list
-        %li.task-list-item
-          %input.task-list-item-checkbox{type: 'checkbox'}
-          Task List Item
-      %textarea.js-task-list-field
-        \- [ ] Task List Item
-
-%form.js-issuable-update{action: '/foo'}

spec/javascripts/issue_spec.js

@@ -4,116 +4,160 @@
 /*= require issue */
 
 (function() {
+  var INVALID_URL = 'http://goesnowhere.nothing/whereami';
+  var $boxClosed, $boxOpen, $btnClose, $btnReopen;
+
+  fixture.preload('issues/closed-issue.html');
+  fixture.preload('issues/issue-with-task-list.html');
+  fixture.preload('issues/open-issue.html');
+
+  function expectErrorMessage() {
+    var $flashMessage = $('div.flash-alert');
+    expect($flashMessage).toExist();
+    expect($flashMessage).toBeVisible();
+    expect($flashMessage).toHaveText('Unable to update this issue at this time.');
+  }
+
+  function expectIssueState(isIssueOpen) {
+    expectVisibility($boxClosed, !isIssueOpen);
+    expectVisibility($boxOpen, isIssueOpen);
+
+    expectVisibility($btnClose, isIssueOpen);
+    expectVisibility($btnReopen, !isIssueOpen);
+  }
+
+  function expectPendingRequest(req, $triggeredButton) {
+    expect(req.type).toBe('PUT');
+    expect(req.url).toBe($triggeredButton.attr('href'));
+    expect($triggeredButton).toHaveProp('disabled', true);
+  }
+
+  function expectVisibility($element, shouldBeVisible) {
+    if (shouldBeVisible) {
+      expect($element).not.toHaveClass('hidden');
+    } else {
+      expect($element).toHaveClass('hidden');
+    }
+  }
+
+  function findElements() {
+      $boxClosed = $('div.status-box-closed');
+      expect($boxClosed).toExist();
+      expect($boxClosed).toHaveText('Closed');
+
+      $boxOpen = $('div.status-box-open');
+      expect($boxOpen).toExist();
+      expect($boxOpen).toHaveText('Open');
+
+      $btnClose = $('.btn-close.btn-grouped');
+      expect($btnClose).toExist();
+      expect($btnClose).toHaveText('Close issue');
+
+      $btnReopen = $('.btn-reopen.btn-grouped');
+      expect($btnReopen).toExist();
+      expect($btnReopen).toHaveText('Reopen issue');
+  }
+
   describe('Issue', function() {
-    return describe('task lists', function() {
-      fixture.preload('issues_show.html');
+    describe('task lists', function() {
+      fixture.load('issues/issue-with-task-list.html');
       beforeEach(function() {
-        fixture.load('issues_show.html');
-        return this.issue = new Issue();
+        this.issue = new Issue();
       });
+
       it('modifies the Markdown field', function() {
         spyOn(jQuery, 'ajax').and.stub();
         $('input[type=checkbox]').attr('checked', true).trigger('change');
-        return expect($('.js-task-list-field').val()).toBe('- [x] Task List Item');
+        expect($('.js-task-list-field').val()).toBe('- [x] Task List Item');
       });
-      return it('submits an ajax request on tasklist:changed', function() {
+      
+      it('submits an ajax request on tasklist:changed', function() {
         spyOn(jQuery, 'ajax').and.callFake(function(req) {
           expect(req.type).toBe('PATCH');
-          expect(req.url).toBe('/foo');
-          return expect(req.data.issue.description).not.toBe(null);
+          expect(req.url).toBe('https://fixture.invalid/namespace3/project3/issues/1.json');
+          expect(req.data.issue.description).not.toBe(null);
         });
-        return $('.js-task-list-field').trigger('tasklist:changed');
+
+        $('.js-task-list-field').trigger('tasklist:changed');
       });
     });
   });
 
-  describe('reopen/close issue', function() {
-    fixture.preload('issues_show.html');
+  describe('close issue', function() {
     beforeEach(function() {
-      fixture.load('issues_show.html');
-      return this.issue = new Issue();
+      fixture.load('issues/open-issue.html');
+      findElements();
+      this.issue = new Issue();
+
+      expectIssueState(true);
     });
+
     it('closes an issue', function() {
-      var $btnClose, $btnReopen;
       spyOn(jQuery, 'ajax').and.callFake(function(req) {
-        expect(req.type).toBe('PUT');
-        expect(req.url).toBe('http://gitlab.com/issues/6/close');
-        return req.success({
+        expectPendingRequest(req, $btnClose);
+        req.success({
           id: 34
         });
       });
-      $btnClose = $('a.btn-close');
-      $btnReopen = $('a.btn-reopen');
-      expect($btnReopen).toBeHidden();
-      expect($btnClose.text()).toBe('Close');
-      expect(typeof $btnClose.prop('disabled')).toBe('undefined');
+
       $btnClose.trigger('click');
-      expect($btnReopen).toBeVisible();
-      expect($btnClose).toBeHidden();
-      expect($('div.status-box-closed')).toBeVisible();
-      return expect($('div.status-box-open')).toBeHidden();
+
+      expectIssueState(false);
+      expect($btnClose).toHaveProp('disabled', false);
     });
+
     it('fails to close an issue with success:false', function() {
-      var $btnClose, $btnReopen;
       spyOn(jQuery, 'ajax').and.callFake(function(req) {
-        expect(req.type).toBe('PUT');
-        expect(req.url).toBe('http://goesnowhere.nothing/whereami');
-        return req.success({
+        expectPendingRequest(req, $btnClose);
+        req.success({
           saved: false
         });
       });
-      $btnClose = $('a.btn-close');
-      $btnReopen = $('a.btn-reopen');
-      $btnClose.attr('href', 'http://goesnowhere.nothing/whereami');
-      expect($btnReopen).toBeHidden();
-      expect($btnClose.text()).toBe('Close');
-      expect(typeof $btnClose.prop('disabled')).toBe('undefined');
+
+      $btnClose.attr('href', INVALID_URL);
       $btnClose.trigger('click');
-      expect($btnReopen).toBeHidden();
-      expect($btnClose).toBeVisible();
-      expect($('div.status-box-closed')).toBeHidden();
-      expect($('div.status-box-open')).toBeVisible();
-      expect($('div.flash-alert')).toBeVisible();
-      return expect($('div.flash-alert').text()).toBe('Unable to update this issue at this time.');
+
+      expectIssueState(true);
+      expect($btnClose).toHaveProp('disabled', false);
+      expectErrorMessage();
     });
+
     it('fails to closes an issue with HTTP error', function() {
-      var $btnClose, $btnReopen;
       spyOn(jQuery, 'ajax').and.callFake(function(req) {
-        expect(req.type).toBe('PUT');
-        expect(req.url).toBe('http://goesnowhere.nothing/whereami');
-        return req.error();
+        expectPendingRequest(req, $btnClose);
+        req.error();
       });
-      $btnClose = $('a.btn-close');
-      $btnReopen = $('a.btn-reopen');
-      $btnClose.attr('href', 'http://goesnowhere.nothing/whereami');
-      expect($btnReopen).toBeHidden();
-      expect($btnClose.text()).toBe('Close');
-      expect(typeof $btnClose.prop('disabled')).toBe('undefined');
+
+      $btnClose.attr('href', INVALID_URL);
       $btnClose.trigger('click');
-      expect($btnReopen).toBeHidden();
-      expect($btnClose).toBeVisible();
-      expect($('div.status-box-closed')).toBeHidden();
-      expect($('div.status-box-open')).toBeVisible();
-      expect($('div.flash-alert')).toBeVisible();
-      return expect($('div.flash-alert').text()).toBe('Unable to update this issue at this time.');
+
+      expectIssueState(true);
+      expect($btnClose).toHaveProp('disabled', true);
+      expectErrorMessage();
     });
-    return it('reopens an issue', function() {
-      var $btnClose, $btnReopen;
+  });
+
+  describe('reopen issue', function() {
+    beforeEach(function() {
+      fixture.load('issues/closed-issue.html');
+      findElements();
+      this.issue = new Issue();
+
+      expectIssueState(false);
+    });
+
+    it('reopens an issue', function() {
       spyOn(jQuery, 'ajax').and.callFake(function(req) {
-        expect(req.type).toBe('PUT');
-        expect(req.url).toBe('http://gitlab.com/issues/6/reopen');
-        return req.success({
+        expectPendingRequest(req, $btnReopen);
+        req.success({
           id: 34
         });
       });
-      $btnClose = $('a.btn-close');
-      $btnReopen = $('a.btn-reopen');
-      expect($btnReopen.text()).toBe('Reopen');
+
       $btnReopen.trigger('click');
-      expect($btnReopen).toBeHidden();
-      expect($btnClose).toBeVisible();
-      expect($('div.status-box-open')).toBeVisible();
-      return expect($('div.status-box-closed')).toBeHidden();
+
+      expectIssueState(true);
+      expect($btnReopen).toHaveProp('disabled', false);
     });
   });
 

spec/spec_helper.rb

@@ -9,7 +9,7 @@ require 'shoulda/matchers'
 require 'sidekiq/testing/inline'
 require 'rspec/retry'
 
-if ENV['CI']
+if ENV['CI'] && !ENV['NO_KNAPSACK']
   require 'knapsack'
   Knapsack::Adapters::RSpecAdapter.bind
 end

spec/support/javascript_fixtures_helpers.rb

@@ -0,0 +1,45 @@
+require 'fileutils'
+require 'gitlab/popen'
+
+module JavaScriptFixturesHelpers
+  include Gitlab::Popen
+
+  FIXTURE_PATH = 'spec/javascripts/fixtures'
+
+  # Public: Removes all fixture files from given directory
+  #
+  # directory_name - directory of the fixtures (relative to FIXTURE_PATH)
+  #
+  def clean_frontend_fixtures(directory_name)
+    directory_name = File.expand_path(directory_name, FIXTURE_PATH)
+    Dir[File.expand_path('*.html.raw', directory_name)].each do |file_name|
+      FileUtils.rm(file_name)
+    end
+  end
+
+  # Public: Store a response object as fixture file
+  #
+  # response - response object to store
+  # fixture_file_name - file name to store the fixture in (relative to FIXTURE_PATH)
+  #
+  def store_frontend_fixture(response, fixture_file_name)
+    fixture_file_name = File.expand_path(fixture_file_name, FIXTURE_PATH)
+    fixture = response.body
+
+    response_mime_type = Mime::Type.lookup(response.content_type)
+    if response_mime_type.html?
+      doc = Nokogiri::HTML::DocumentFragment.parse(fixture)
+
+      scripts = doc.css('script')
+      scripts.remove
+
+      fixture = doc.to_html
+
+      # replace relative links
+      fixture.gsub!(%r{="/}, '="https://fixture.invalid/')
+    end
+
+    FileUtils.mkdir_p(File.dirname(fixture_file_name))
+    File.write(fixture_file_name, fixture)
+  end
+end