diff options
58 files changed, 1605 insertions, 1285 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6b261bd5938..83e41a11e52 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -296,9 +296,9 @@ might be edited to make them small and simple. Please submit Feature Proposals using the ['Feature Proposal' issue template](.gitlab/issue_templates/Feature Proposal.md) provided on the issue tracker. -For changes in the interface, it is helpful to include a mockup. Issues that add to, or change, the interface should -be given the ~"UX" label. This will allow the UX team to provide input and guidance. You may -need to ask one of the [core team] members to add the label, if you do not have permissions to do it by yourself. +For changes in the interface, it is helpful to include a mockup. Issues that add to, or change, the interface should +be given the ~"UX" label. This will allow the UX team to provide input and guidance. You may +need to ask one of the [core team] members to add the label, if you do not have permissions to do it by yourself. If you want to create something yourself, consider opening an issue first to discuss whether it is interesting to include this in GitLab. @@ -658,7 +658,7 @@ available at [http://contributor-covenant.org/version/1/1/0/](http://contributor [license-finder-doc]: doc/development/licensing.md [GitLab Inc engineering workflow]: https://about.gitlab.com/handbook/engineering/workflow/#labelling-issues [polling-etag]: https://docs.gitlab.com/ce/development/polling.html -[testing]: doc/development/testing.md +[testing]: doc/development/testing_guide/index.md [^1]: Please note that specs other than JavaScript specs are considered backend code. diff --git a/PROCESS.md b/PROCESS.md index 5e65bb59246..06963243b25 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -1,4 +1,4 @@ -## GitLab Core Team & GitLab Inc. Contribution Process +## GitLab core team & GitLab Inc. contribution process --- diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index d43eae79730..5ebc92110dd 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -29,7 +29,8 @@ import CILintEditor from './ci_lint_editor'; /* global ProjectNew */ /* global ProjectShow */ /* global ProjectImport */ -/* global Labels */ +import Labels from './labels'; +import LabelManager from './label_manager'; /* global Shortcuts */ /* global ShortcutsFindFile */ /* global Sidebar */ @@ -459,7 +460,7 @@ import U2FAuthenticate from './u2f/authenticate'; case 'groups:labels:index': case 'projects:labels:index': if ($('.prioritized-labels').length) { - new gl.LabelManager(); + new LabelManager(); } $('.label-subscription').each((i, el) => { const $el = $(el); diff --git a/app/assets/javascripts/label_manager.js b/app/assets/javascripts/label_manager.js index a8f613c6cf2..c929dc98c10 100644 --- a/app/assets/javascripts/label_manager.js +++ b/app/assets/javascripts/label_manager.js @@ -3,123 +3,119 @@ import Flash from './flash'; -((global) => { - class LabelManager { - constructor({ togglePriorityButton, prioritizedLabels, otherLabels } = {}) { - this.togglePriorityButton = togglePriorityButton || $('.js-toggle-priority'); - this.prioritizedLabels = prioritizedLabels || $('.js-prioritized-labels'); - this.otherLabels = otherLabels || $('.js-other-labels'); - this.errorMessage = 'Unable to update label prioritization at this time'; - this.emptyState = document.querySelector('#js-priority-labels-empty-state'); - this.sortable = Sortable.create(this.prioritizedLabels.get(0), { - filter: '.empty-message', - forceFallback: true, - fallbackClass: 'is-dragging', - dataIdAttr: 'data-id', - onUpdate: this.onPrioritySortUpdate.bind(this), - }); - this.bindEvents(); - } +export default class LabelManager { + constructor({ togglePriorityButton, prioritizedLabels, otherLabels } = {}) { + this.togglePriorityButton = togglePriorityButton || $('.js-toggle-priority'); + this.prioritizedLabels = prioritizedLabels || $('.js-prioritized-labels'); + this.otherLabels = otherLabels || $('.js-other-labels'); + this.errorMessage = 'Unable to update label prioritization at this time'; + this.emptyState = document.querySelector('#js-priority-labels-empty-state'); + this.sortable = Sortable.create(this.prioritizedLabels.get(0), { + filter: '.empty-message', + forceFallback: true, + fallbackClass: 'is-dragging', + dataIdAttr: 'data-id', + onUpdate: this.onPrioritySortUpdate.bind(this), + }); + this.bindEvents(); + } - bindEvents() { - this.prioritizedLabels.find('.btn-action').on('mousedown', this, this.onButtonActionClick); - return this.togglePriorityButton.on('click', this, this.onTogglePriorityClick); - } + bindEvents() { + this.prioritizedLabels.find('.btn-action').on('mousedown', this, this.onButtonActionClick); + return this.togglePriorityButton.on('click', this, this.onTogglePriorityClick); + } - onTogglePriorityClick(e) { - e.preventDefault(); - const _this = e.data; - const $btn = $(e.currentTarget); - const $label = $(`#${$btn.data('domId')}`); - const action = $btn.parents('.js-prioritized-labels').length ? 'remove' : 'add'; - const $tooltip = $(`#${$btn.find('.has-tooltip:visible').attr('aria-describedby')}`); - $tooltip.tooltip('destroy'); - _this.toggleLabelPriority($label, action); - _this.toggleEmptyState($label, $btn, action); - } + onTogglePriorityClick(e) { + e.preventDefault(); + const _this = e.data; + const $btn = $(e.currentTarget); + const $label = $(`#${$btn.data('domId')}`); + const action = $btn.parents('.js-prioritized-labels').length ? 'remove' : 'add'; + const $tooltip = $(`#${$btn.find('.has-tooltip:visible').attr('aria-describedby')}`); + $tooltip.tooltip('destroy'); + _this.toggleLabelPriority($label, action); + _this.toggleEmptyState($label, $btn, action); + } - onButtonActionClick(e) { - e.stopPropagation(); - $(e.currentTarget).tooltip('hide'); - } + onButtonActionClick(e) { + e.stopPropagation(); + $(e.currentTarget).tooltip('hide'); + } - toggleEmptyState($label, $btn, action) { - this.emptyState.classList.toggle('hidden', !!this.prioritizedLabels[0].querySelector(':scope > li')); - } + toggleEmptyState($label, $btn, action) { + this.emptyState.classList.toggle('hidden', !!this.prioritizedLabels[0].querySelector(':scope > li')); + } - toggleLabelPriority($label, action, persistState) { - if (persistState == null) { - persistState = true; - } - let xhr; - const _this = this; - const url = $label.find('.js-toggle-priority').data('url'); - let $target = this.prioritizedLabels; - let $from = this.otherLabels; - if (action === 'remove') { - $target = this.otherLabels; - $from = this.prioritizedLabels; - } - $label.detach().appendTo($target); - if ($from.find('li').length) { - $from.find('.empty-message').removeClass('hidden'); - } - if ($target.find('> li:not(.empty-message)').length) { - $target.find('.empty-message').addClass('hidden'); - } - // Return if we are not persisting state - if (!persistState) { - return; - } - if (action === 'remove') { - xhr = $.ajax({ - url, - type: 'DELETE' - }); - // Restore empty message - if (!$from.find('li').length) { - $from.find('.empty-message').removeClass('hidden'); - } - } else { - xhr = this.savePrioritySort($label, action); - } - return xhr.fail(this.rollbackLabelPosition.bind(this, $label, action)); + toggleLabelPriority($label, action, persistState) { + if (persistState == null) { + persistState = true; } - - onPrioritySortUpdate() { - const xhr = this.savePrioritySort(); - return xhr.fail(function() { - return new Flash(this.errorMessage, 'alert'); - }); + let xhr; + const _this = this; + const url = $label.find('.js-toggle-priority').data('url'); + let $target = this.prioritizedLabels; + let $from = this.otherLabels; + if (action === 'remove') { + $target = this.otherLabels; + $from = this.prioritizedLabels; } - - savePrioritySort() { - return $.post({ - url: this.prioritizedLabels.data('url'), - data: { - label_ids: this.getSortedLabelsIds() - } + $label.detach().appendTo($target); + if ($from.find('li').length) { + $from.find('.empty-message').removeClass('hidden'); + } + if ($target.find('> li:not(.empty-message)').length) { + $target.find('.empty-message').addClass('hidden'); + } + // Return if we are not persisting state + if (!persistState) { + return; + } + if (action === 'remove') { + xhr = $.ajax({ + url, + type: 'DELETE' }); + // Restore empty message + if (!$from.find('li').length) { + $from.find('.empty-message').removeClass('hidden'); + } + } else { + xhr = this.savePrioritySort($label, action); } + return xhr.fail(this.rollbackLabelPosition.bind(this, $label, action)); + } - rollbackLabelPosition($label, originalAction) { - const action = originalAction === 'remove' ? 'add' : 'remove'; - this.toggleLabelPriority($label, action, false); + onPrioritySortUpdate() { + const xhr = this.savePrioritySort(); + return xhr.fail(function() { return new Flash(this.errorMessage, 'alert'); - } + }); + } - getSortedLabelsIds() { - const sortedIds = []; - this.prioritizedLabels.find('> li').each(function() { - const id = $(this).data('id'); + savePrioritySort() { + return $.post({ + url: this.prioritizedLabels.data('url'), + data: { + label_ids: this.getSortedLabelsIds() + } + }); + } - if (id) { - sortedIds.push(id); - } - }); - return sortedIds; - } + rollbackLabelPosition($label, originalAction) { + const action = originalAction === 'remove' ? 'add' : 'remove'; + this.toggleLabelPriority($label, action, false); + return new Flash(this.errorMessage, 'alert'); } - gl.LabelManager = LabelManager; -})(window.gl || (window.gl = {})); + getSortedLabelsIds() { + const sortedIds = []; + this.prioritizedLabels.find('> li').each(function() { + const id = $(this).data('id'); + + if (id) { + sortedIds.push(id); + } + }); + return sortedIds; + } +} diff --git a/app/assets/javascripts/labels.js b/app/assets/javascripts/labels.js index 03dd61b4263..7aab13ed9c6 100644 --- a/app/assets/javascripts/labels.js +++ b/app/assets/javascripts/labels.js @@ -1,44 +1,35 @@ -/* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, vars-on-top, no-unused-vars, max-len */ -(function() { - this.Labels = (function() { - function Labels() { - this.setSuggestedColor = this.setSuggestedColor.bind(this); - this.updateColorPreview = this.updateColorPreview.bind(this); - var form; - form = $('.label-form'); - this.cleanBinding(); - this.addBinding(); - this.updateColorPreview(); - } +export default class Labels { + constructor() { + this.setSuggestedColor = this.setSuggestedColor.bind(this); + this.updateColorPreview = this.updateColorPreview.bind(this); + this.cleanBinding(); + this.addBinding(); + this.updateColorPreview(); + } - Labels.prototype.addBinding = function() { - $(document).on('click', '.suggest-colors a', this.setSuggestedColor); - return $(document).on('input', 'input#label_color', this.updateColorPreview); - }; + addBinding() { + $(document).on('click', '.suggest-colors a', this.setSuggestedColor); + return $(document).on('input', 'input#label_color', this.updateColorPreview); + } + // eslint-disable-next-line class-methods-use-this + cleanBinding() { + $(document).off('click', '.suggest-colors a'); + return $(document).off('input', 'input#label_color'); + } + // eslint-disable-next-line class-methods-use-this + updateColorPreview() { + const previewColor = $('input#label_color').val(); + return $('div.label-color-preview').css('background-color', previewColor); + // Updates the the preview color with the hex-color input + } - Labels.prototype.cleanBinding = function() { - $(document).off('click', '.suggest-colors a'); - return $(document).off('input', 'input#label_color'); - }; - - Labels.prototype.updateColorPreview = function() { - var previewColor; - previewColor = $('input#label_color').val(); - return $('div.label-color-preview').css('background-color', previewColor); - // Updates the the preview color with the hex-color input - }; - - // Updates the preview color with a click on a suggested color - Labels.prototype.setSuggestedColor = function(e) { - var color; - color = $(e.currentTarget).data('color'); - $('input#label_color').val(color); - this.updateColorPreview(); - // Notify the form, that color has changed - $('.label-form').trigger('keyup'); - return e.preventDefault(); - }; - - return Labels; - })(); -}).call(window); + // Updates the preview color with a click on a suggested color + setSuggestedColor(e) { + const color = $(e.currentTarget).data('color'); + $('input#label_color').val(color); + this.updateColorPreview(); + // Notify the form, that color has changed + $('.label-form').trigger('keyup'); + return e.preventDefault(); + } +} diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 50aa445e9e7..8d7608ce0f4 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -79,8 +79,6 @@ import './issuable_context'; import './issuable_form'; import './issue'; import './issue_status_select'; -import './label_manager'; -import './labels'; import './labels_select'; import './layout_nav'; import LazyLoader from './lazy_loader'; @@ -118,7 +116,6 @@ import './right_sidebar'; import './search'; import './search_autocomplete'; import './smart_interval'; -import './star'; import './subscription'; import './subscription_select'; import initBreadcrumbs from './breadcrumb'; diff --git a/app/assets/javascripts/projects/project_new.js b/app/assets/javascripts/projects/project_new.js index 7f972b6f6ee..3ecc0c2a6e5 100644 --- a/app/assets/javascripts/projects/project_new.js +++ b/app/assets/javascripts/projects/project_new.js @@ -29,6 +29,12 @@ const bindEvents = () => { const $newProjectForm = $('#new_project'); const $projectImportUrl = $('#project_import_url'); const $projectPath = $('#project_path'); + const $useTemplateBtn = $('.template-button > input'); + const $projectFieldsForm = $('.project-fields-form'); + const $selectedTemplateText = $('.selected-template'); + const $changeTemplateBtn = $('.change-template'); + const $selectedIcon = $('.selected-icon svg'); + const $templateProjectNameInput = $('#template-project-name #project_path'); if ($newProjectForm.length !== 1) { return; @@ -48,6 +54,40 @@ const bindEvents = () => { $('.btn_import_gitlab_project').attr('href', `${importHref}?namespace_id=${$('#project_namespace_id').val()}&path=${$projectPath.val()}`); }); + function chooseTemplate() { + $('.template-option').hide(); + $projectFieldsForm.addClass('selected'); + $selectedIcon.removeClass('active'); + const value = $(this).val(); + const templates = { + rails: { + text: 'Ruby on Rails', + icon: '.selected-icon .icon-rails', + }, + express: { + text: 'NodeJS Express', + icon: '.selected-icon .icon-node-express', + }, + spring: { + text: 'Spring', + icon: '.selected-icon .icon-java-spring', + }, + }; + + const selectedTemplate = templates[value]; + $selectedTemplateText.text(selectedTemplate.text); + $(selectedTemplate.icon).addClass('active'); + $templateProjectNameInput.focus(); + } + + $useTemplateBtn.on('change', chooseTemplate); + + $changeTemplateBtn.on('click', () => { + $('.template-option').show(); + $projectFieldsForm.removeClass('selected'); + $useTemplateBtn.prop('checked', false); + }); + $newProjectForm.on('submit', () => { $projectPath.val($projectPath.val().trim()); }); diff --git a/app/assets/javascripts/star.js b/app/assets/javascripts/star.js index 77db075d1ef..1a8dc085772 100644 --- a/app/assets/javascripts/star.js +++ b/app/assets/javascripts/star.js @@ -1,27 +1,29 @@ -/* eslint-disable func-names, space-before-function-paren, wrap-iife, no-unused-vars, one-var, no-var, one-var-declaration-per-line, prefer-arrow-callback, no-new, max-len */ import Flash from './flash'; import { __, s__ } from './locale'; export default class Star { constructor() { - $('.project-home-panel .toggle-star').on('ajax:success', function(e, data, status, xhr) { - var $starIcon, $starSpan, $this, toggleStar; - $this = $(this); - $starSpan = $this.find('span'); - $starIcon = $this.find('i'); - toggleStar = function(isStarred) { - $this.parent().find('.star-count').text(data.star_count); - if (isStarred) { - $starSpan.removeClass('starred').text(s__('StarProject|Star')); - $starIcon.removeClass('fa-star').addClass('fa-star-o'); - } else { - $starSpan.addClass('starred').text(__('Unstar')); - $starIcon.removeClass('fa-star-o').addClass('fa-star'); + $('.project-home-panel .toggle-star') + .on('ajax:success', function handleSuccess(e, data) { + const $this = $(this); + const $starSpan = $this.find('span'); + const $starIcon = $this.find('i'); + + function toggleStar(isStarred) { + $this.parent().find('.star-count').text(data.star_count); + if (isStarred) { + $starSpan.removeClass('starred').text(s__('StarProject|Star')); + $starIcon.removeClass('fa-star').addClass('fa-star-o'); + } else { + $starSpan.addClass('starred').text(__('Unstar')); + $starIcon.removeClass('fa-star-o').addClass('fa-star'); + } } - }; - toggleStar($starSpan.hasClass('starred')); - }).on('ajax:error', function(e, xhr, status, error) { - new Flash('Star toggle failed. Try again later.', 'alert'); - }); + + toggleStar($starSpan.hasClass('starred')); + }) + .on('ajax:error', () => { + Flash('Star toggle failed. Try again later.', 'alert'); + }); } } diff --git a/app/assets/stylesheets/framework.scss b/app/assets/stylesheets/framework.scss index c7be94e2c8e..0d7a5cba928 100644 --- a/app/assets/stylesheets/framework.scss +++ b/app/assets/stylesheets/framework.scss @@ -38,6 +38,7 @@ @import "framework/new-sidebar"; @import "framework/tables"; @import "framework/notes"; +@import "framework/tabs"; @import "framework/timeline"; @import "framework/tooltips"; @import "framework/typography"; diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index 5b950ae0ba0..9dcf332eee2 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -749,7 +749,7 @@ margin-bottom: $dropdown-vertical-offset; } - li:not(.dropdown-bold-header) { + li { display: block; padding: 0 1px; diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index 22945e935ef..d79444fad79 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -161,9 +161,10 @@ } } - .dropdown-bold-header { + li.dropdown-bold-header { color: $gl-text-color-secondary; font-size: 12px; + padding: 0 16px; } .navbar-collapse { diff --git a/app/assets/stylesheets/framework/selects.scss b/app/assets/stylesheets/framework/selects.scss index 50f1445bc2e..58f6e62b06a 100644 --- a/app/assets/stylesheets/framework/selects.scss +++ b/app/assets/stylesheets/framework/selects.scss @@ -61,6 +61,7 @@ border: 1px solid $dropdown-border-color; min-width: 175px; color: $gl-text-color; + z-index: 999; } .select2-drop.select2-drop-above.select2-drop-active { diff --git a/app/assets/stylesheets/framework/tabs.scss b/app/assets/stylesheets/framework/tabs.scss new file mode 100644 index 00000000000..c8ba14b7066 --- /dev/null +++ b/app/assets/stylesheets/framework/tabs.scss @@ -0,0 +1,35 @@ +.gitlab-tabs { + background: $gray-light; + border: 1px solid $border-color; + + li { + width: 50%; + + &:not(:last-child) { + border-right: 1px solid $border-color; + } + + &.active { + background: $white-light; + } + + a { + width: 100%; + text-align: center; + } + } +} + +.gitlab-tab-content { + border: 1px solid $border-color; + border-top: 0; + margin-bottom: $gl-padding; + + .tab-pane { + padding: $gl-padding; + + &.no-padding { + padding: 0; + } + } +} diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index bbbd16322eb..089a67a7c98 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -699,14 +699,6 @@ $perf-bar-bucket-color: #ccc; $perf-bar-bucket-box-shadow-from: rgba($white-light, .2); $perf-bar-bucket-box-shadow-to: rgba($black, .25); - -/* -Project Templates Icons -*/ -$rails: #c00; -$node: #353535; -$java: #70ad51; - /* Issuable warning */ diff --git a/app/assets/stylesheets/pages/members.scss b/app/assets/stylesheets/pages/members.scss index b3bab082a35..692acf74a58 100644 --- a/app/assets/stylesheets/pages/members.scss +++ b/app/assets/stylesheets/pages/members.scss @@ -3,41 +3,12 @@ border-bottom: 1px solid $border-color; } -.project-member-tabs { - background: $gray-light; - border: 1px solid $border-color; - - li { - width: 50%; - - &.active { - background: $white-light; - } - - &:first-child { - border-right: 1px solid $border-color; - } - - a { - width: 100%; - text-align: center; - } - } -} - .users-project-form { .btn-create { margin-right: 10px; } } -.project-member-tab-content { - padding: $gl-padding; - border: 1px solid $border-color; - border-top: 0; - margin-bottom: $gl-padding; -} - .member { .list-item-name { @media (min-width: $screen-sm-min) { diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index a086c11324d..61dc9f13d50 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -48,7 +48,8 @@ border: 1px solid $border-color; } - + .select2 a { + + .select2 a, + + .btn-default { border-top-left-radius: 0; border-bottom-left-radius: 0; } @@ -549,10 +550,92 @@ a.deploy-project-label { } } -.project-template, +.project-template { + > .form-group { + margin-bottom: 0; + } + + .template-option { + padding: $gl-padding $gl-padding $gl-padding ($gl-padding * 4); + position: relative; + + &:not(:first-child) { + border-top: 1px solid $border-color; + } + } + + .template-title { + font-size: 16px; + } + + .template-description { + margin: 6px 0 12px; + } + + .template-button { + input { + position: absolute; + clip: rect(0, 0, 0, 0); + } + } + + svg { + position: absolute; + left: $gl-padding; + top: $gl-padding; + } + + .project-fields-form { + display: none; + + &.selected { + display: block; + padding: $gl-padding; + } + } + + .template-input-group { + position: relative; + + @media (min-width: $screen-sm-min) { + display: flex; + } + + .input-group-addon { + flex: 1; + text-align: left; + padding-left: ($gl-padding * 3); + background-color: $white-light; + } + + .selected-template { + line-height: 20px; + } + + .selected-icon { + svg { + display: none; + top: 7px; + height: 20px; + width: 20px; + + &.active { + display: block; + } + } + } + } +} + +.gitlab-tab-content { + .import-project-pane { + padding-bottom: 6px; + } +} + .project-import { .form-group { - margin-bottom: 5px; + margin-bottom: 0; } .import-buttons { @@ -567,10 +650,6 @@ a.deploy-project-label { margin-right: 10px; } - .blank-option { - min-width: 70px; - } - .btn-template-icon { height: 24px; width: inherit; @@ -592,18 +671,6 @@ a.deploy-project-label { } } - .icon-rails path { - fill: $rails; - } - - .icon-node-express path { - fill: $node; - } - - .icon-java-spring path { - fill: $java; - } - > div { margin-bottom: 10px; padding-left: 0; @@ -611,10 +678,6 @@ a.deploy-project-label { } } -.project-templates-buttons .btn:last-child { - margin-right: 0; -} - .create-project-options { display: flex; @@ -1053,6 +1116,12 @@ pre.light-well { min-width: 100px; } + &.form-group { + @media (min-width: $screen-sm-min) { + margin-bottom: 0; + } + } + .select2-choice { border-top-right-radius: 0; border-bottom-right-radius: 0; diff --git a/app/controllers/admin/runners_controller.rb b/app/controllers/admin/runners_controller.rb index 719893c0bc8..38b808cdc31 100644 --- a/app/controllers/admin/runners_controller.rb +++ b/app/controllers/admin/runners_controller.rb @@ -2,7 +2,8 @@ class Admin::RunnersController < Admin::ApplicationController before_action :runner, except: :index def index - @runners = Ci::Runner.order('id DESC') + sort = params[:sort] == 'contacted_asc' ? { contacted_at: :asc } : { id: :desc } + @runners = Ci::Runner.order(sort) @runners = @runners.search(params[:search]) if params[:search].present? @runners = @runners.page(params[:page]).per(30) @active_runners_cnt = Ci::Runner.online.count diff --git a/app/views/admin/runners/index.html.haml b/app/views/admin/runners/index.html.haml index 43cea1358cc..76f4a817744 100644 --- a/app/views/admin/runners/index.html.haml +++ b/app/views/admin/runners/index.html.haml @@ -63,7 +63,7 @@ %th Projects %th Jobs %th Tags - %th Last contact + %th= link_to 'Last contact', admin_runners_path(params.slice(:search).merge(sort: 'contacted_asc')) %th - @runners.each do |runner| diff --git a/app/views/projects/_new_project_fields.html.haml b/app/views/projects/_new_project_fields.html.haml new file mode 100644 index 00000000000..a78a8e5d628 --- /dev/null +++ b/app/views/projects/_new_project_fields.html.haml @@ -0,0 +1,41 @@ +- visibility_level = params.dig(:project, :visibility_level) || default_project_visibility + +.row{ id: project_name_id } + .form-group.project-path.col-sm-6 + = f.label :namespace_id, class: 'label-light' do + %span + Project path + .input-group + - if current_user.can_select_namespace? + .input-group-addon + = root_url + = f.select :namespace_id, namespaces_options(namespace_id_from(params) || :current_user, display_path: true, extra_group: namespace_id_from(params)), {}, { class: 'select2 js-select-namespace', tabindex: 1} + + - else + .input-group-addon.static-namespace + #{user_url(current_user.username)}/ + = f.hidden_field :namespace_id, value: current_user.namespace_id + .form-group.project-path.col-sm-6 + = f.label :path, class: 'label-light' do + %span + Project name + = f.text_field :path, placeholder: "my-awesome-project", class: "form-control", tabindex: 2, autofocus: true, required: true +- if current_user.can_create_group? + .help-block + Want to house several dependent projects under the same namespace? + = link_to "Create a group", new_group_path + +.form-group + = f.label :description, class: 'label-light' do + Project description + %span (optional) + = f.text_area :description, placeholder: 'Description format', class: "form-control", rows: 3, maxlength: 250 + +.form-group.visibility-level-setting + = f.label :visibility_level, class: 'label-light' do + Visibility Level + = link_to icon('question-circle'), help_page_path("public_access/public_access"), aria: { label: 'Documentation for Visibility Level' } + = render 'shared/visibility_level', f: f, visibility_level: visibility_level.to_i, can_change_visibility_level: true, form_model: @project, with_label: false + += f.submit 'Create project', class: "btn btn-create project-submit", tabindex: 4 += link_to 'Cancel', dashboard_projects_path, class: 'btn btn-cancel' diff --git a/app/views/projects/_project_templates.html.haml b/app/views/projects/_project_templates.html.haml index 5638b7da1b0..d50175727be 100644 --- a/app/views/projects/_project_templates.html.haml +++ b/app/views/projects/_project_templates.html.haml @@ -1,10 +1,24 @@ -.project-templates-buttons.import-buttons{ data: { toggle: "buttons" } } - .btn.blank-option.active - %input{ type: "radio", autocomplete: "off", name: "project[template_name]", id: "blank", checked: "true", value: "" } - = icon('file-o', class: 'btn-template-icon') - Blank +.project-templates-buttons.import-buttons - Gitlab::ProjectTemplate.all.each do |template| - .btn - %input{ type: "radio", autocomplete: "off", name: "project[template_name]", id: template.name, value: template.name } + .template-option = custom_icon(template.logo) - = template.title + .template-title= template.title + .template-description= template.description + %label.btn.btn-success.template-button.choose-template.append-right-10{ for: template.name } + %input{ type: "radio", autocomplete: "off", name: "project[template_name]", id: template.name, value: template.name } + %span Use template + %a.btn.btn-default{ href: template.preview, rel: 'noopener noreferrer', target: '_blank' } Preview + + .project-fields-form + .form-group + %label.label-light + Template + .input-group.template-input-group + .input-group-addon + .selected-icon + - Gitlab::ProjectTemplate.all.each do |template| + = custom_icon(template.logo) + .selected-template + %button.btn.btn-default.change-template{ type: "button" } Change template + + = render 'new_project_fields', f: f, project_name_id: "template-project-name" diff --git a/app/views/projects/new.html.haml b/app/views/projects/new.html.haml index cc41b908946..0934c47a8e2 100644 --- a/app/views/projects/new.html.haml +++ b/app/views/projects/new.html.haml @@ -14,24 +14,40 @@ .col-lg-3.profile-settings-sidebar %h4.prepend-top-0 New project - - if import_sources_enabled? - %p - A project is where you house your files (repository), plan your work (issues), and publish your documentation (wiki), #{link_to 'among other things', help_page_path("user/project/index.md", anchor: "projects-features"), target: '_blank'}. - %p - All features are enabled when you create a project, but you can disable the ones you don’t need in the project settings. + %p + A project is where you house your files (repository), plan your work (issues), and publish your documentation (wiki), #{link_to 'among other things', help_page_path("user/project/index.md", anchor: "projects-features"), target: '_blank'}. + %p + All features are enabled when you create a project, but you can disable the ones you don’t need in the project settings. .col-lg-9.js-toggle-container - = form_for @project, html: { class: 'new_project' } do |f| - .create-project-options - .first-column + %ul.nav-links.gitlab-tabs{ role: 'tablist' } + %li.active{ role: 'presentation' } + %a{ href: '#blank-project-pane', id: 'blank-project-tab', data: { toggle: 'tab' }, role: 'tab' } + %span.hidden-xs Blank project + %span.visible-xs Blank + %li{ role: 'presentation' } + %a{ href: '#create-from-template-pane', id: 'create-from-template-tab', data: { toggle: 'tab' }, role: 'tab' } + %span.hidden-xs Create from template + %span.visible-xs Template + %li{ role: 'presentation' } + %a{ href: '#import-project-pane', id: 'import-project-tab', data: { toggle: 'tab' }, role: 'tab' } + %span.hidden-xs Import project + %span.visible-xs Import + + .tab-content.gitlab-tab-content + .tab-pane.active{ id: 'blank-project-pane', role: 'tabpanel' } + = form_for @project, html: { class: 'new_project' } do |f| + = render 'new_project_fields', f: f, project_name_id: "blank-project-name" + + .tab-pane.no-padding{ id: 'create-from-template-pane', role: 'tabpanel' } + = form_for @project, html: { class: 'new_project' } do |f| .project-template .form-group - = f.label :template_project, class: 'label-light' do - Create from template - = link_to icon('question-circle'), help_page_path("gitlab-basics/create-project"), target: '_blank', aria: { label: "What’s included in a template?" }, title: "What’s included in a template?", class: 'has-tooltip', data: { placement: 'top'} %div = render 'project_templates', f: f - - if import_sources_enabled? - .second-column + + .tab-pane.import-project-pane{ id: 'import-project-pane', role: 'tabpanel' } + = form_for @project, html: { class: 'new_project' } do |f| + - if import_sources_enabled? .project-import .form-group.clearfix = f.label :visibility_level, class: 'label-light' do #the label here seems wrong @@ -74,54 +90,10 @@ .import_gitlab_project.has-tooltip{ data: { container: 'body' } } = link_to new_import_gitlab_project_path, class: 'btn btn_import_gitlab_project project-submit' do = icon('gitlab', text: 'GitLab export') - - .row - .col-lg-12 - .js-toggle-content.hide - %hr - = render "shared/import_form", f: f - %hr - - .row - .form-group.col-xs-12.col-sm-6 - = f.label :namespace_id, class: 'label-light' do - %span - Project path - .form-group - .input-group - - if current_user.can_select_namespace? - .input-group-addon - = root_url - = f.select :namespace_id, namespaces_options(namespace_id_from(params) || :current_user, display_path: true, extra_group: namespace_id_from(params)), {}, { class: 'select2 js-select-namespace', tabindex: 1} - - - else - .input-group-addon.static-namespace - #{root_url}#{current_user.username}/ - = f.hidden_field :namespace_id, value: current_user.namespace_id - .form-group.col-xs-12.col-sm-6.project-path - = f.label :path, class: 'label-light' do - %span - Project name - = f.text_field :path, placeholder: "my-awesome-project", class: "form-control", tabindex: 2, autofocus: true, required: true - - if current_user.can_create_group? - .help-block - Want to house several dependent projects under the same namespace? - = link_to "Create a group", new_group_path - - .form-group - = f.label :description, class: 'label-light' do - Project description - %span.light (optional) - = f.text_area :description, placeholder: 'Description format', class: "form-control", rows: 3, maxlength: 250 - - .form-group.visibility-level-setting - = f.label :visibility_level, class: 'label-light' do - Visibility Level - = link_to icon('question-circle'), help_page_path("public_access/public_access"), aria: { label: 'Documentation for Visibility Level' } - = render 'shared/visibility_level', f: f, visibility_level: visibility_level.to_i, can_change_visibility_level: true, form_model: @project, with_label: false - - = f.submit 'Create project', class: "btn btn-create project-submit", tabindex: 4 - = link_to 'Cancel', dashboard_projects_path, class: 'btn btn-cancel' + .col-lg-12 + .js-toggle-content.hide + %hr + = render "shared/import_form", f: f .save-project-loader.hide .center diff --git a/app/views/projects/project_members/index.html.haml b/app/views/projects/project_members/index.html.haml index 25153fd0b6f..fd5d3ec56da 100644 --- a/app/views/projects/project_members/index.html.haml +++ b/app/views/projects/project_members/index.html.haml @@ -17,14 +17,14 @@ %i Owners .light - if can?(current_user, :admin_project_member, @project) - %ul.nav-links.project-member-tabs{ role: 'tablist' } + %ul.nav-links.gitlab-tabs{ role: 'tablist' } %li.active{ role: 'presentation' } %a{ href: '#add-member-pane', id: 'add-member-tab', data: { toggle: 'tab' }, role: 'tab' } Add member - if @project.allowed_to_share_with_group? %li{ role: 'presentation' } %a{ href: '#share-with-group-pane', id: 'share-with-group-tab', data: { toggle: 'tab' }, role: 'tab' } Share with group - .tab-content.project-member-tab-content + .tab-content.gitlab-tab-content .tab-pane.active{ id: 'add-member-pane', role: 'tabpanel' } = render 'projects/project_members/new_project_member', tab_title: 'Add member' .tab-pane{ id: 'share-with-group-pane', role: 'tabpanel' } diff --git a/app/views/shared/icons/_express.svg b/app/views/shared/icons/_express.svg index f2c94319f19..a51e81e5568 100644 --- a/app/views/shared/icons/_express.svg +++ b/app/views/shared/icons/_express.svg @@ -1,6 +1 @@ -<svg xmlns="http://www.w3.org/2000/svg" width="27" height="32" viewBox="0 0 27 32" class="btn-template-icon icon-node-express"> - <g fill="none" fill-rule="evenodd" transform="translate(-3)"> - <rect width="32" height="32"/> - <path fill="#353535" d="M4.19170065,16.2667139 C4.23142421,18.3323387 4.47969269,20.2489714 4.93651356,22.0166696 C5.39333443,23.7843677 6.09841693,25.3236323 7.05178222,26.6345096 C8.00514751,27.9453869 9.23655921,28.9781838 10.7460543,29.7329313 C12.2555493,30.4876788 14.1026668,30.8650469 16.2874623,30.8650469 C19.5050701,30.8650469 22.1764391,30.0209341 24.3016492,28.3326831 C26.4268593,26.644432 27.7476477,24.1120935 28.2640539,20.7355914 L29.4557545,20.7355914 C29.0187954,24.3107112 27.6086304,27.0813875 25.2252172,29.0477034 C22.841804,31.0140194 19.9023051,31.9971626 16.4066324,31.9971626 C14.0232191,32.0368861 11.9874175,31.659518 10.2991665,30.8650469 C8.61091547,30.0705759 7.23054269,28.9484023 6.15800673,27.4984926 C5.08547078,26.0485829 4.29101162,24.3404957 3.77460543,22.3741798 C3.25819923,20.4078639 3,18.2926164 3,16.0283738 C3,13.4860664 3.3773681,11.2218578 4.13211562,9.23568007 C4.88686314,7.24950238 5.87993709,5.57120741 7.11136726,4.20074481 C8.34279742,2.8302822 9.77282391,1.78755456 11.4014896,1.07253059 C13.0301553,0.357506621 14.6985195,0 16.4066324,0 C18.7900456,0 20.8457087,0.456814016 22.5736832,1.37045575 C24.3016578,2.28409749 25.7118228,3.4956477 26.8042206,5.00514275 C27.8966183,6.51463779 28.6910775,8.24258646 29.1876219,10.1890406 C29.6841663,12.1354947 29.8927118,14.1613656 29.8132647,16.2667139 L4.19170065,16.2667139 Z M28.6215641,15.0750133 C28.6215641,13.2080062 28.3633648,11.4304039 27.8469586,9.74215285 C27.3305524,8.05390181 26.5658855,6.57422163 25.5529349,5.30306791 C24.5399843,4.03191419 23.2787803,3.0289095 21.7692853,2.29402376 C20.2597903,1.55913801 18.5119801,1.19170065 16.5258024,1.19170065 C14.8574132,1.19170065 13.2982871,1.50948432 11.8483774,2.14506118 C10.3984676,2.78063804 9.12733299,3.70419681 8.03493526,4.9157652 C6.94253754,6.12733359 6.05870172,7.58715229 5.38340131,9.2952651 C4.70810089,11.0033779 4.31087132,12.9299414 4.19170065,15.0750133 L28.6215641,15.0750133 Z"/> - </g> -</svg> +<svg xmlns="http://www.w3.org/2000/svg" width="27" height="32" viewBox="0 0 27 32" class="btn-template-icon icon-node-express"><g fill="none" fill-rule="evenodd"><path d="M-3 0h32v32H-3z"/><path fill="#353535" d="M1.192 16.267c.04 2.065.288 3.982.745 5.75.456 1.767 1.16 3.307 2.115 4.618.953 1.31 2.185 2.343 3.694 3.098 1.51.755 3.357 1.132 5.54 1.132 3.22 0 5.89-.844 8.016-2.532 2.125-1.69 3.446-4.22 3.962-7.597h1.192c-.437 3.575-1.847 6.345-4.23 8.312-2.384 1.966-5.324 2.95-8.82 2.95-2.383.04-4.42-.338-6.107-1.133-1.69-.794-3.07-1.917-4.142-3.367-1.073-1.45-1.867-3.158-2.383-5.124C.258 20.408 0 18.294 0 16.028c0-2.542.377-4.806 1.132-6.792C1.887 7.25 2.88 5.57 4.112 4.2 5.34 2.83 6.77 1.79 8.4 1.074 10.03.358 11.698 0 13.406 0c2.383 0 4.44.457 6.167 1.37 1.728.914 3.138 2.126 4.23 3.635 1.093 1.51 1.887 3.238 2.384 5.184.496 1.945.705 3.97.625 6.077H1.193zm24.43-1.192c0-1.867-.26-3.645-.775-5.333-.516-1.688-1.28-3.168-2.294-4.44-1.013-1.27-2.274-2.273-3.784-3.008-1.51-.735-3.258-1.102-5.244-1.102-1.67 0-3.228.317-4.678.953-1.45.636-2.72 1.56-3.813 2.77-1.092 1.212-1.976 2.672-2.652 4.38-.675 1.708-1.072 3.635-1.19 5.78h24.43z"/></g></svg> diff --git a/app/views/shared/icons/_rails.svg b/app/views/shared/icons/_rails.svg index 0bb09a705df..852bd183cc7 100644 --- a/app/views/shared/icons/_rails.svg +++ b/app/views/shared/icons/_rails.svg @@ -1,6 +1 @@ -<svg xmlns="http://www.w3.org/2000/svg" width="32" height="20" viewBox="0 0 32 20" class="btn-template-icon icon-rails"> - <g fill="none" fill-rule="evenodd" transform="translate(0 -6)"> - <rect width="32" height="32"/> - <path fill="#C00" fill-rule="nonzero" d="M0.984615385,25.636044 C0.984615385,25.636044 1.40659341,21.4725275 4.36043956,16.5494505 C7.31428571,11.6263736 12.3498901,7.8989011 16.4430769,7.53318681 C24.5872527,6.71736264 31.9015385,14.0175824 31.9015385,14.0175824 C31.9015385,14.0175824 31.6624176,14.1863736 31.4092308,14.3973626 C23.4197802,8.48967033 18.5389011,11.2747253 17.0057143,12.0202198 C9.97274725,15.9446154 12.0967033,25.636044 12.0967033,25.636044 L0.984615385,25.636044 Z M24.1371429,8.32087912 C23.687033,8.13802198 23.2369231,7.96923077 22.7727473,7.81450549 L22.829011,6.88615385 C23.7151648,7.13934066 24.0668132,7.30813187 24.1934066,7.37846154 L24.1371429,8.32087912 Z M22.8008791,11.3028571 C23.250989,11.330989 23.7151648,11.3872527 24.1934066,11.4857143 L24.1371429,12.3578022 C23.672967,12.2593407 23.2087912,12.2030769 22.7446154,12.189011 L22.8008791,11.3028571 Z M17.5964835,6.91428571 C17.1885714,6.91428571 16.7806593,6.92835165 16.3727473,6.97054945 L16.1054945,6.14065934 C16.5696703,6.0843956 17.0197802,6.05626374 17.4558242,6.05626374 L17.7371429,6.91428571 C17.6949451,6.91428571 17.6386813,6.91428571 17.5964835,6.91428571 Z M18.2716484,12.0905495 C18.6232967,11.9358242 19.0312088,11.7810989 19.5094505,11.6404396 L19.8189011,12.5687912 C19.410989,12.6953846 19.0030769,12.8641758 18.5951648,13.0610989 L18.2716484,12.0905495 Z M11.8857143,8.39120879 C11.52,8.57406593 11.1683516,8.78505495 10.8026374,9.01010989 L10.1556044,8.02549451 C10.5353846,7.80043956 10.9010989,7.60351648 11.2527473,7.42065934 L11.8857143,8.39120879 Z M14.7692308,14.7208791 C15.0224176,14.3973626 15.3178022,14.0738462 15.6413187,13.7784615 L16.2742857,14.7349451 C15.9648352,15.0584615 15.6835165,15.381978 15.4443956,15.7336264 L14.7692308,14.7208791 Z M12.7296703,19.2501099 C12.8421978,18.7437363 12.9687912,18.2232967 13.1516484,17.7028571 L14.1643956,18.5046154 C14.0237363,19.0531868 13.9252747,19.6017582 13.869011,20.1503297 L12.7296703,19.2501099 Z M6.56879121,12.5687912 C6.23120879,12.9204396 5.90769231,13.3002198 5.61230769,13.68 L4.52923077,12.7516484 C4.85274725,12.4 5.2043956,12.0483516 5.57010989,11.6967033 L6.56879121,12.5687912 Z M2.32087912,18.8562637 C2.09582418,19.3767033 1.80043956,20.0659341 1.61758242,20.5441758 L0,19.9534066 C0.140659341,19.5736264 0.436043956,18.8703297 0.703296703,18.2654945 L2.32087912,18.8562637 Z M12.5186813,22.8228571 L14.0378022,23.3714286 C14.1221978,24.0325275 14.2487912,24.6514286 14.3753846,25.2 L12.6874725,24.5951648 C12.6171429,24.1731868 12.5468132,23.5683516 12.5186813,22.8228571 Z"/> - </g> -</svg> +<svg xmlns="http://www.w3.org/2000/svg" width="32" height="20" viewBox="0 0 32 20" class="btn-template-icon icon-rails"><g fill="none" fill-rule="evenodd"><path d="M0-6h32v32H0z"/><path fill="#c00" fill-rule="nonzero" d="M.985 19.636s.422-4.163 3.375-9.087c2.954-4.924 7.99-8.65 12.083-9.017 8.144-.816 15.46 6.485 15.46 6.485s-.24.168-.494.38C23.42 2.49 18.54 5.274 17.005 6.02c-7.033 3.925-4.91 13.616-4.91 13.616H.987zM24.137 2.32c-.45-.182-.9-.35-1.364-.505l.056-.93c.885.254 1.237.423 1.363.493l-.056.943zM22.8 5.304c.45.028.915.084 1.393.183l-.056.872c-.464-.1-.928-.155-1.392-.17l.056-.885zM17.597.913c-.407 0-.815.015-1.223.058l-.268-.83c.465-.056.915-.084 1.35-.084l.282.858h-.14zm.676 5.178c.35-.154.76-.31 1.237-.45l.31.93c-.41.125-.817.294-1.225.49l-.323-.97zm-6.386-3.7c-.366.184-.718.395-1.083.62l-.647-.985c.38-.225.745-.42 1.097-.604l.633.97zm2.883 6.33c.252-.323.548-.646.87-.942l.634.957c-.31.323-.59.647-.83 1L14.77 8.72zm-2.04 4.53c.112-.506.24-1.027.422-1.547l1.012.802c-.14.548-.24 1.097-.295 1.645l-1.14-.9zM6.57 6.57c-.34.35-.662.73-.958 1.11L4.53 6.752c.323-.352.674-.704 1.04-1.055l1 .872zm-4.25 6.286c-.224.52-.52 1.21-.702 1.688L0 13.954c.14-.38.436-1.084.703-1.69l1.618.592zm10.2 3.967l1.518.548c.084.663.21 1.28.337 1.83l-1.688-.605c-.07-.422-.14-1.027-.168-1.772z"/></g></svg> diff --git a/app/views/shared/icons/_spring.svg b/app/views/shared/icons/_spring.svg index 508349aa456..ccf18749029 100644 --- a/app/views/shared/icons/_spring.svg +++ b/app/views/shared/icons/_spring.svg @@ -1,6 +1 @@ -<svg xmlns="http://www.w3.org/2000/svg" width="32" height="32" viewBox="0 0 32 32" class="btn-template-icon icon-java-spring"> - <g fill="none" fill-rule="evenodd"> - <rect width="32" height="32"/> - <path fill="#70AD51" d="M5.46647617,27.9932117 C6.0517027,28.4658996 6.91159892,28.3777063 7.38425926,27.7914452 C7.85922261,27.2048452 7.76991326,26.3449044 7.18398981,25.8699411 C6.59874295,25.3956543 5.74015536,25.4869934 5.26383884,26.0722403 C4.81393367,26.6267596 4.87238621,27.4284565 5.37913494,27.9159868 L5.11431334,27.6818383 C1.97157151,24.7616933 0,20.5966301 0,15.9782542 C0,7.16842834 7.16775175,0 15.9796074,0 C20.4586065,0 24.5113565,1.8565519 27.4145869,4.8362365 C28.0749348,3.93840692 28.6466499,2.93435335 29.115524,1.82069284 C31.1513712,7.93770658 32.3482517,13.0811131 31.909824,17.1311567 C31.3178113,25.4044499 24.4017495,31.9585382 15.9796074,31.9585382 C12.0682639,31.9585382 8.48438805,30.5444735 5.7042963,28.2034861 L5.46647617,27.9932117 Z M29.0471888,23.0106888 C33.0546075,17.6737787 30.8211972,9.04527781 28.9612624,3.529749 C27.3029502,6.98304378 23.2217836,9.62375882 19.6981239,10.4613722 C16.3950312,11.2482417 13.4715032,10.6021021 10.4153644,11.7780085 C3.44517575,14.457289 3.55613585,22.7698242 7.39373146,24.6365249 C7.39711439,24.6392312 7.62444728,24.7616933 7.62174094,24.7576338 C7.62309411,24.7562806 13.2658211,23.6358542 16.3862356,22.4843049 C20.9450718,20.7996058 25.9524846,16.6494275 27.5986182,11.8273993 C26.723116,16.8415779 22.4179995,21.6669891 18.093262,23.8828081 C15.7908399,25.0648038 14.0005934,25.3279957 10.2123886,26.6385428 C9.74892722,26.798217 9.38492397,26.9538318 9.38492397,26.9538318 C10.3463526,26.7948341 11.301692,26.7420604 11.301692,26.7420604 C16.6954354,26.4869875 25.1087819,28.2582896 29.0471888,23.0106888 Z"/> - </g> -</svg> +<svg xmlns="http://www.w3.org/2000/svg" width="32" height="32" viewBox="0 0 32 32" class="btn-template-icon icon-java-spring"><g fill="none" fill-rule="evenodd"><path d="M0 0h32v32H0z"/><path fill="#70AD51" d="M5.466 27.993c.586.473 1.446.385 1.918-.202.475-.585.386-1.445-.2-1.92-.585-.474-1.444-.383-1.92.202-.45.555-.392 1.356.115 1.844l-.266-.234C1.972 24.762 0 20.597 0 15.978 0 7.168 7.168 0 15.98 0c4.48 0 8.53 1.857 11.435 4.836.66-.898 1.232-1.902 1.7-3.015 2.036 6.118 3.233 11.26 2.795 15.31-.592 8.274-7.508 14.83-15.93 14.83-3.912 0-7.496-1.416-10.276-3.757l-.238-.21zm23.58-4.982c4.01-5.336 1.775-13.965-.085-19.48-1.657 3.453-5.738 6.094-9.262 6.93-3.303.788-6.226.142-9.283 1.318-6.97 2.68-6.86 10.992-3.02 12.86.002 0 .23.124.227.12 0-.002 5.644-1.122 8.764-2.274 4.56-1.684 9.566-5.835 11.213-10.657-.877 5.015-5.182 9.84-9.507 12.056-2.302 1.182-4.092 1.445-7.88 2.756-.464.158-.828.314-.828.314.96-.16 1.917-.212 1.917-.212 5.393-.255 13.807 1.516 17.745-3.73z"/></g></svg> diff --git a/changelogs/unreleased/36160-select2-dropdown.yml b/changelogs/unreleased/36160-select2-dropdown.yml new file mode 100644 index 00000000000..a836744fb41 --- /dev/null +++ b/changelogs/unreleased/36160-select2-dropdown.yml @@ -0,0 +1,5 @@ +--- +title: Decreases z-index of select2 to a lower number of our navigation bar +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/38720-sort-admin-runners.yml b/changelogs/unreleased/38720-sort-admin-runners.yml new file mode 100644 index 00000000000..b1047644891 --- /dev/null +++ b/changelogs/unreleased/38720-sort-admin-runners.yml @@ -0,0 +1,5 @@ +--- +title: Add sort runners on admin runners +merge_request: 14661 +author: Takuya Noguchi +type: added diff --git a/config/routes/google_api.rb b/config/routes/google_api.rb index 3fb236d3d51..a119b47c176 100644 --- a/config/routes/google_api.rb +++ b/config/routes/google_api.rb @@ -1,5 +1,7 @@ -namespace :google_api do - resource :auth, only: [], controller: :authorizations do - match :callback, via: [:get, :post] +scope '-' do + namespace :google_api do + resource :auth, only: [], controller: :authorizations do + match :callback, via: [:get, :post] + end end end diff --git a/doc/development/README.md b/doc/development/README.md index b648c7ce086..e2d0c6c2056 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -1,75 +1,89 @@ -# Development +# GitLab development guides -## Outside of docs +## Get started! -- [CONTRIBUTING.md](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md) main contributing guide -- [PROCESS.md](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/PROCESS.md) contributing process -- [GitLab Development Kit (GDK)](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/README.md) to install a development version +- Setup GitLab's development environment with [GitLab Development Kit (GDK)](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/README.md) +- [GitLab contributing guide](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md) +- [Architecture](architecture.md) of GitLab +- [Rake tasks](rake_tasks.md) for development -## Styleguides +## Processes + +- [GitLab core team & GitLab Inc. contribution process](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/PROCESS.md) +- [Generate a changelog entry with `bin/changelog`](changelog.md) +- [Code review guidelines](code_review.md) for reviewing code and having code reviewed. +- [Limit conflicts with EE when developing on CE](limit_ee_conflicts.md) + +## UX and frontend guides -- [API styleguide](api_styleguide.md) Use this styleguide if you are - contributing to the API. -- [Documentation styleguide](doc_styleguide.md) Use this styleguide if you are - contributing to documentation. -- [Writing documentation](writing_documentation.md) - - [Distinction between general documentation and technical articles](writing_documentation.md#distinction-between-general-documentation-and-technical-articles) -- [SQL Migration Style Guide](migration_style_guide.md) for creating safe SQL migrations -- [Testing standards and style guidelines](testing.md) - [UX guide](ux_guide/index.md) for building GitLab with existing CSS styles and elements - [Frontend guidelines](fe_guide/index.md) -- [SQL guidelines](sql.md) for working with SQL queries + +## Backend guides + +- [Testing standards and style guidelines](testing_guide/index.md) +- [API styleguide](api_styleguide.md) Use this styleguide if you are + contributing to the API. - [Sidekiq guidelines](sidekiq_style_guide.md) for working with Sidekiq workers +- [Working with Gitaly](gitaly.md) +- [Manage feature flags](feature_flags.md) +- [View sent emails or preview mailers](emails.md) +- [Shell commands](shell_commands.md) in the GitLab codebase - [`Gemfile` guidelines](gemfile.md) +- [Sidekiq debugging](sidekiq_debugging.md) +- [Gotchas](gotchas.md) to avoid +- [Issue and merge requests state models](object_state_models.md) +- [How to dump production data to staging](db_dump.md) -## Process +## Performance guides -- [Generate a changelog entry with `bin/changelog`](changelog.md) -- [Limit conflicts with EE when developing on CE](limit_ee_conflicts.md) -- [Code review guidelines](code_review.md) for reviewing code and having code reviewed. +- [Instrumentation](instrumentation.md) +- [Performance guidelines](performance.md) - [Merge request performance guidelines](merge_request_performance_guidelines.md) for ensuring merge requests do not negatively impact GitLab performance -## Backend howtos - -- [Architecture](architecture.md) of GitLab -- [Gotchas](gotchas.md) to avoid -- [How to dump production data to staging](db_dump.md) -- [Instrumentation](instrumentation.md) -- [Performance guidelines](performance.md) -- [Rake tasks](rake_tasks.md) for development -- [Shell commands](shell_commands.md) in the GitLab codebase -- [Sidekiq debugging](sidekiq_debugging.md) -- [Object state models](object_state_models.md) -- [Building a package for testing purposes](build_test_package.md) -- [Manage feature flags](feature_flags.md) -- [View sent emails or preview mailers](emails.md) -- [Working with Gitaly](gitaly.md) +## Databases guides -## Databases +### Migrations -- [Merge Request Checklist](database_merge_request_checklist.md) - [What requires downtime?](what_requires_downtime.md) +- [SQL guidelines](sql.md) for working with SQL queries +- [Migrations style guide](migration_style_guide.md) for creating safe SQL migrations +- [Post deployment migrations](post_deployment_migrations.md) +- [Background migrations](background_migrations.md) +- [Swapping tables](swapping_tables.md) + +### Best practices + +- [Merge Request checklist](database_merge_request_checklist.md) - [Adding database indexes](adding_database_indexes.md) -- [Post Deployment Migrations](post_deployment_migrations.md) -- [Foreign Keys & Associations](foreign_keys.md) -- [Serializing Data](serializing_data.md) -- [Polymorphic Associations](polymorphic_associations.md) -- [Single Table Inheritance](single_table_inheritance.md) -- [Background Migrations](background_migrations.md) -- [Storing SHA1 Hashes As Binary](sha1_as_binary.md) -- [Iterating Tables In Batches](iterating_tables_in_batches.md) -- [Ordering Table Columns](ordering_table_columns.md) -- [Verifying Database Capabilities](verifying_database_capabilities.md) -- [Hash Indexes](hash_indexes.md) -- [Swapping Tables](swapping_tables.md) - -## Internationalization (i18n) +- [Foreign keys & associations](foreign_keys.md) +- [Single table inheritance](single_table_inheritance.md) +- [Polymorphic associations](polymorphic_associations.md) +- [Serializing data](serializing_data.md) +- [Hash indexes](hash_indexes.md) +- [Storing SHA1 hashes as binary](sha1_as_binary.md) +- [Iterating tables in batches](iterating_tables_in_batches.md) +- [Ordering table columns](ordering_table_columns.md) +- [Verifying database capabilities](verifying_database_capabilities.md) + +## Documentation guides + +- [Documentation styleguide](doc_styleguide.md): Use this styleguide if you are + contributing to the documentation. +- [Writing documentation](writing_documentation.md) + - [Distinction between general documentation and technical articles](writing_documentation.md#distinction-between-general-documentation-and-technical-articles) + +## Internationalization (i18n) guides - [Introduction](i18n/index.md) - [Externalization](i18n/externalization.md) - [Translation](i18n/translation.md) +## Build guides + +- [Building a package for testing purposes](build_test_package.md) + ## Compliance - [Licensing](licensing.md) for ensuring license compliance diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 031b12a8e91..c0e1bfc12a1 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -54,7 +54,8 @@ or make changes to our frontend development guidelines. --- -## [Testing](testing.md) +## [Testing](../testing_guide/frontend_testing.md) + How we write frontend tests, run the GitLab test suite, and debug test related issues. diff --git a/doc/development/fe_guide/testing.md b/doc/development/fe_guide/testing.md index 867c83f1e72..98e499b8c0f 100644 --- a/doc/development/fe_guide/testing.md +++ b/doc/development/fe_guide/testing.md @@ -1,254 +1 @@ -# Frontend Testing - -There are two types of test suites you'll encounter while developing frontend code -at GitLab. We use Karma and Jasmine for JavaScript unit and integration testing, and RSpec -feature tests with Capybara for e2e (end-to-end) integration testing. - -Unit and feature tests need to be written for all new features. -Most of the time, you should use rspec for your feature tests. -There are cases where the behaviour you are testing is not worth the time spent running the full application, -for example, if you are testing styling, animation, edge cases or small actions that don't involve the backend, -you should write an integration test using Jasmine. - - - -_This diagram demonstrates the relative priority of each test type we use_ - -Regression tests should be written for bug fixes to prevent them from recurring in the future. - -See [the Testing Standards and Style Guidelines](../testing.md) -for more information on general testing practices at GitLab. - -## Karma test suite - -GitLab uses the [Karma][karma] test runner with [Jasmine][jasmine] as its test -framework for our JavaScript unit and integration tests. For integration tests, -we generate HTML files using RSpec (see `spec/javascripts/fixtures/*.rb` for examples). -Some fixtures are still HAML templates that are translated to HTML files using the same mechanism (see `static_fixtures.rb`). -Adding these static fixtures should be avoided as they are harder to keep up to date with real views. -The existing static fixtures will be migrated over time. -Please see [gitlab-org/gitlab-ce#24753](https://gitlab.com/gitlab-org/gitlab-ce/issues/24753) to track our progress. -Fixtures are served during testing by the [jasmine-jquery][jasmine-jquery] plugin. - -JavaScript tests live in `spec/javascripts/`, matching the folder structure -of `app/assets/javascripts/`: `app/assets/javascripts/behaviors/autosize.js` -has a corresponding `spec/javascripts/behaviors/autosize_spec.js` file. - -Keep in mind that in a CI environment, these tests are run in a headless -browser and you will not have access to certain APIs, such as -[`Notification`](https://developer.mozilla.org/en-US/docs/Web/API/notification), -which will have to be stubbed. - -### Best practice - -#### Naming unit tests - -When writing describe test blocks to test specific functions/methods, -please use the method name as the describe block name. - -```javascript -// Good -describe('methodName', () => { - it('passes', () => { - expect(true).toEqual(true); - }); -}); - -// Bad -describe('#methodName', () => { - it('passes', () => { - expect(true).toEqual(true); - }); -}); - -// Bad -describe('.methodName', () => { - it('passes', () => { - expect(true).toEqual(true); - }); -}); -``` -#### Testing Promises - -When testing Promises you should always make sure that the test is asynchronous and rejections are handled. -Your Promise chain should therefore end with a call of the `done` callback and `done.fail` in case an error occurred. - -```javascript -// Good -it('tests a promise', (done) => { - promise - .then((data) => { - expect(data).toBe(asExpected); - }) - .then(done) - .catch(done.fail); -}); - -// Good -it('tests a promise rejection', (done) => { - promise - .then(done.fail) - .catch((error) => { - expect(error).toBe(expectedError); - }) - .then(done) - .catch(done.fail); -}); - -// Bad (missing done callback) -it('tests a promise', () => { - promise - .then((data) => { - expect(data).toBe(asExpected); - }) -}); - -// Bad (missing catch) -it('tests a promise', (done) => { - promise - .then((data) => { - expect(data).toBe(asExpected); - }) - .then(done) -}); - -// Bad (use done.fail in asynchronous tests) -it('tests a promise', (done) => { - promise - .then((data) => { - expect(data).toBe(asExpected); - }) - .then(done) - .catch(fail) -}); - -// Bad (missing catch) -it('tests a promise rejection', (done) => { - promise - .catch((error) => { - expect(error).toBe(expectedError); - }) - .then(done) -}); -``` - -#### Stubbing - -For unit tests, you should stub methods that are unrelated to the current unit you are testing. -If you need to use a prototype method, instantiate an instance of the class and call it there instead of mocking the instance completely. - -For integration tests, you should stub methods that will effect the stability of the test if they -execute their original behaviour. i.e. Network requests. - -### Vue.js unit tests -See this [section][vue-test]. - -### Running frontend tests - -`rake karma` runs the frontend-only (JavaScript) tests. -It consists of two subtasks: - -- `rake karma:fixtures` (re-)generates fixtures -- `rake karma:tests` actually executes the tests - -As long as the fixtures don't change, `rake karma:tests` (or `yarn karma`) -is sufficient (and saves you some time). - -### Live testing and focused testing - -While developing locally, it may be helpful to keep karma running so that you -can get instant feedback on as you write tests and modify code. To do this -you can start karma with `npm run karma-start`. It will compile the javascript -assets and run a server at `http://localhost:9876/` where it will automatically -run the tests on any browser which connects to it. You can enter that url on -multiple browsers at once to have it run the tests on each in parallel. - -While karma is running, any changes you make will instantly trigger a recompile -and retest of the entire test suite, so you can see instantly if you've broken -a test with your changes. You can use [jasmine focused][jasmine-focus] or -excluded tests (with `fdescribe` or `xdescribe`) to get karma to run only the -tests you want while you're working on a specific feature, but make sure to -remove these directives when you commit your code. - -## RSpec Feature Integration Tests - -Information on setting up and running RSpec integration tests with -[Capybara][capybara] can be found in the -[general testing guide](../testing.md). - -## Gotchas - -### Errors due to use of unsupported JavaScript features - -Similar errors will be thrown if you're using JavaScript features not yet -supported by the PhantomJS test runner which is used for both Karma and RSpec -tests. We polyfill some JavaScript objects for older browsers, but some -features are still unavailable: - -- Array.from -- Array.first -- Async functions -- Generators -- Array destructuring -- For..Of -- Symbol/Symbol.iterator -- Spread - -Until these are polyfilled appropriately, they should not be used. Please -update this list with additional unsupported features. - -### RSpec errors due to JavaScript - -By default RSpec unit tests will not run JavaScript in the headless browser -and will simply rely on inspecting the HTML generated by rails. - -If an integration test depends on JavaScript to run correctly, you need to make -sure the spec is configured to enable JavaScript when the tests are run. If you -don't do this you'll see vague error messages from the spec runner. - -To enable a JavaScript driver in an `rspec` test, add `:js` to the -individual spec or the context block containing multiple specs that need -JavaScript enabled: - -```ruby -# For one spec -it 'presents information about abuse report', :js do - # assertions... -end - -describe "Admin::AbuseReports", :js do - it 'presents information about abuse report' do - # assertions... - end - it 'shows buttons for adding to abuse report' do - # assertions... - end -end -``` - -### Spinach errors due to missing JavaScript - -> **Note:** Since we are discouraging the use of Spinach when writing new -> feature tests, you shouldn't ever need to use this. This information is kept -> available for legacy purposes only. - -In Spinach, the JavaScript driver is enabled differently. In the `*.feature` -file for the failing spec, add the `@javascript` flag above the Scenario: - -``` -@javascript -Scenario: Developer can approve merge request - Given I am a "Shop" developer - And I visit project "Shop" merge requests page - And merge request 'Bug NS-04' must be approved - And I click link "Bug NS-04" - When I click link "Approve" - Then I should see approved merge request "Bug NS-04" -``` - -[capybara]: http://teamcapybara.github.io/capybara/ -[jasmine]: https://jasmine.github.io/ -[jasmine-focus]: https://jasmine.github.io/2.5/focused_specs.html -[jasmine-jquery]: https://github.com/velesin/jasmine-jquery -[karma]: http://karma-runner.github.io/ -[vue-test]:https://docs.gitlab.com/ce/development/fe_guide/vue.html#testing-vue-components +This document was moved to [../testing_guide/frontend_testing.md](../testing_guide/frontend_testing.md). diff --git a/doc/development/testing.md b/doc/development/testing.md index 4d5b90de6fc..45b1519ece8 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -1,566 +1 @@ -# Testing Standards and Style Guidelines - -This guide outlines standards and best practices for automated testing of GitLab -CE and EE. - -It is meant to be an _extension_ of the [thoughtbot testing -styleguide](https://github.com/thoughtbot/guides/tree/master/style/testing). If -this guide defines a rule that contradicts the thoughtbot guide, this guide -takes precedence. Some guidelines may be repeated verbatim to stress their -importance. - -## Definitions - -### Unit tests - -Formal definition: https://en.wikipedia.org/wiki/Unit_testing - -These kind of tests ensure that a single unit of code (a method) works as -expected (given an input, it has a predictable output). These tests should be -isolated as much as possible. For example, model methods that don't do anything -with the database shouldn't need a DB record. Classes that don't need database -records should use stubs/doubles as much as possible. - -| Code path | Tests path | Testing engine | Notes | -| --------- | ---------- | -------------- | ----- | -| `app/finders/` | `spec/finders/` | RSpec | | -| `app/helpers/` | `spec/helpers/` | RSpec | | -| `app/db/{post_,}migrate/` | `spec/migrations/` | RSpec | More details at [`spec/migrations/README.md`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/migrations/README.md). | -| `app/policies/` | `spec/policies/` | RSpec | | -| `app/presenters/` | `spec/presenters/` | RSpec | | -| `app/routing/` | `spec/routing/` | RSpec | | -| `app/serializers/` | `spec/serializers/` | RSpec | | -| `app/services/` | `spec/services/` | RSpec | | -| `app/tasks/` | `spec/tasks/` | RSpec | | -| `app/uploaders/` | `spec/uploaders/` | RSpec | | -| `app/views/` | `spec/views/` | RSpec | | -| `app/workers/` | `spec/workers/` | RSpec | | -| `app/assets/javascripts/` | `spec/javascripts/` | Karma | More details in the [JavaScript](#javascript) section. | - -### Integration tests - -Formal definition: https://en.wikipedia.org/wiki/Integration_testing - -These kind of tests ensure that individual parts of the application work well together, without the overhead of the actual app environment (i.e. the browser). These tests should assert at the request/response level: status code, headers, body. They're useful to test permissions, redirections, what view is rendered etc. - -| Code path | Tests path | Testing engine | Notes | -| --------- | ---------- | -------------- | ----- | -| `app/controllers/` | `spec/controllers/` | RSpec | | -| `app/mailers/` | `spec/mailers/` | RSpec | | -| `lib/api/` | `spec/requests/api/` | RSpec | | -| `lib/ci/api/` | `spec/requests/ci/api/` | RSpec | | -| `app/assets/javascripts/` | `spec/javascripts/` | Karma | More details in the [JavaScript](#javascript) section. | - -#### About controller tests - -In an ideal world, controllers should be thin. However, when this is not the -case, it's acceptable to write a system/feature test without JavaScript instead -of a controller test. The reason is that testing a fat controller usually -involves a lot of stubbing, things like: - -```ruby -controller.instance_variable_set(:@user, user) -``` - -and use methods which are deprecated in Rails 5 ([#23768]). - -[#23768]: https://gitlab.com/gitlab-org/gitlab-ce/issues/23768 - -#### About Karma - -As you may have noticed, Karma is both in the Unit tests and the Integration -tests category. That's because Karma is a tool that provides an environment to -run JavaScript tests, so you can either run unit tests (e.g. test a single -JavaScript method), or integration tests (e.g. test a component that is composed -of multiple components). - -### System tests or Feature tests - -Formal definition: https://en.wikipedia.org/wiki/System_testing. - -These kind of tests ensure the application works as expected from a user point -of view (aka black-box testing). These tests should test a happy path for a -given page or set of pages, and a test case should be added for any regression -that couldn't have been caught at lower levels with better tests (i.e. if a -regression is found, regression tests should be added at the lowest-level -possible). - -| Tests path | Testing engine | Notes | -| ---------- | -------------- | ----- | -| `spec/features/` | [Capybara] + [RSpec] | If your spec has the `:js` metadata, the browser driver will be [Poltergeist], otherwise it's using [RackTest]. | -| `features/` | Spinach | Spinach tests are deprecated, [you shouldn't add new Spinach tests](#spinach-feature-tests). | - -[Capybara]: https://github.com/teamcapybara/capybara -[RSpec]: https://github.com/rspec/rspec-rails#feature-specs -[Poltergeist]: https://github.com/teamcapybara/capybara#poltergeist -[RackTest]: https://github.com/teamcapybara/capybara#racktest - -#### Best practices - -- Create only the necessary records in the database -- Test a happy path and a less happy path but that's it -- Every other possible path should be tested with Unit or Integration tests -- Test what's displayed on the page, not the internals of ActiveRecord models. - For instance, if you want to verify that a record was created, add - expectations that its attributes are displayed on the page, not that - `Model.count` increased by one. -- It's ok to look for DOM elements but don't abuse it since it makes the tests - more brittle - -If we're confident that the low-level components work well (and we should be if -we have enough Unit & Integration tests), we shouldn't need to duplicate their -thorough testing at the System test level. - -It's very easy to add tests, but a lot harder to remove or improve tests, so one -should take care of not introducing too many (slow and duplicated) specs. - -The reasons why we should follow these best practices are as follows: - -- System tests are slow to run since they spin up the entire application stack - in a headless browser, and even slower when they integrate a JS driver -- When system tests run with a JavaScript driver, the tests are run in a - different thread than the application. This means it does not share a - database connection and your test will have to commit the transactions in - order for the running application to see the data (and vice-versa). In that - case we need to truncate the database after each spec instead of simply - rolling back a transaction (the faster strategy that's in use for other kind - of tests). This is slower than transactions, however, so we want to use - truncation only when necessary. - -### Black-box tests or End-to-end tests - -GitLab consists of [multiple pieces] such as [GitLab Shell], [GitLab Workhorse], -[Gitaly], [GitLab Pages], [GitLab Runner], and GitLab Rails. All theses pieces -are configured and packaged by [GitLab Omnibus]. - -[GitLab QA] is a tool that allows to test that all these pieces integrate well -together by building a Docker image for a given version of GitLab Rails and -running feature tests (i.e. using Capybara) against it. - -The actual test scenarios and steps are [part of GitLab Rails] so that they're -always in-sync with the codebase. - -[multiple pieces]: ./architecture.md#components -[GitLab Shell]: https://gitlab.com/gitlab-org/gitlab-shell -[GitLab Workhorse]: https://gitlab.com/gitlab-org/gitlab-workhorse -[Gitaly]: https://gitlab.com/gitlab-org/gitaly -[GitLab Pages]: https://gitlab.com/gitlab-org/gitlab-pages -[GitLab Runner]: https://gitlab.com/gitlab-org/gitlab-ci-multi-runner -[GitLab Omnibus]: https://gitlab.com/gitlab-org/omnibus-gitlab -[GitLab QA]: https://gitlab.com/gitlab-org/gitlab-qa -[part of GitLab Rails]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa - -## Test for what should not be there - -This is particularly important for permission calls and might be called a -negative assertion: make sure only the bare minimum is returned and nothing else. - -See an issue about [leaking tokens] as an example of a vulnerability that is -captured by such a test. - -[leaking tokens]: https://gitlab.com/gitlab-org/gitlab-ce/issues/37948 - -## How to test at the correct level? - -As many things in life, deciding what to test at each level of testing is a -trade-off: - -- Unit tests are usually cheap, and you should consider them like the basement - of your house: you need them to be confident that your code is behaving - correctly. However if you run only unit tests without integration / system - tests, you might [miss] the [big] [picture]! -- Integration tests are a bit more expensive, but don't abuse them. A system test - is often better than an integration test that is stubbing a lot of internals. -- System tests are expensive (compared to unit tests), even more if they require - a JavaScript driver. Make sure to follow the guidelines in the [Speed](#test-speed) - section. - -Another way to see it is to think about the "cost of tests", this is well -explained [in this article][tests-cost] and the basic idea is that the cost of a -test includes: - -- The time it takes to write the test -- The time it takes to run the test every time the suite runs -- The time it takes to understand the test -- The time it takes to fix the test if it breaks and the underlying code is OK -- Maybe, the time it takes to change the code to make the code testable. - -[miss]: https://twitter.com/ThePracticalDev/status/850748070698651649 -[big]: https://twitter.com/timbray/status/822470746773409794 -[picture]: https://twitter.com/withzombies/status/829716565834752000 -[tests-cost]: https://medium.com/table-xi/high-cost-tests-and-high-value-tests-a86e27a54df#.2ulyh3a4e - -## Frontend testing - -Please consult the [dedicated "Frontend testing" guide](./fe_guide/testing.md). - -## RSpec - -### General Guidelines - -- Use a single, top-level `describe ClassName` block. -- Use `.method` to describe class methods and `#method` to describe instance - methods. -- Use `context` to test branching logic. -- Don't assert against the absolute value of a sequence-generated attribute (see [Gotchas](gotchas.md#dont-assert-against-the-absolute-value-of-a-sequence-generated-attribute)). -- Try to match the ordering of tests to the ordering within the class. -- Try to follow the [Four-Phase Test][four-phase-test] pattern, using newlines - to separate phases. -- Use `Gitlab.config.gitlab.host` rather than hard coding `'localhost'` -- Don't assert against the absolute value of a sequence-generated attribute (see - [Gotchas](gotchas.md#dont-assert-against-the-absolute-value-of-a-sequence-generated-attribute)). -- Don't supply the `:each` argument to hooks since it's the default. -- On `before` and `after` hooks, prefer it scoped to `:context` over `:all` - -[four-phase-test]: https://robots.thoughtbot.com/four-phase-test - -### Automatic retries and flaky tests detection - -On our CI, we use [rspec-retry] to automatically retry a failing example a few -times (see [`spec/spec_helper.rb`] for the precise retries count). - -We also use a home-made `RspecFlaky::Listener` listener which records flaky -examples in a JSON report file on `master` (`retrieve-tests-metadata` and `update-tests-metadata` jobs), and warns when a new flaky example -is detected in any other branch (`flaky-examples-check` job). In the future, the -`flaky-examples-check` job will not be allowed to fail. - -[rspec-retry]: https://github.com/NoRedInk/rspec-retry -[`spec/spec_helper.rb`]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/spec_helper.rb - -### `let` variables - -GitLab's RSpec suite has made extensive use of `let` variables to reduce -duplication. However, this sometimes [comes at the cost of clarity][lets-not], -so we need to set some guidelines for their use going forward: - -- `let` variables are preferable to instance variables. Local variables are - preferable to `let` variables. -- Use `let` to reduce duplication throughout an entire spec file. -- Don't use `let` to define variables used by a single test; define them as - local variables inside the test's `it` block. -- Don't define a `let` variable inside the top-level `describe` block that's - only used in a more deeply-nested `context` or `describe` block. Keep the - definition as close as possible to where it's used. -- Try to avoid overriding the definition of one `let` variable with another. -- Don't define a `let` variable that's only used by the definition of another. - Use a helper method instead. - -[lets-not]: https://robots.thoughtbot.com/lets-not - -#### `set` variables - -In some cases there is no need to recreate the same object for tests again for -each example. For example, a project is needed to test issues on the same -project, one project will do for the entire file. This can be achieved by using -`set` in the same way you would use `let`. - -`rspec-set` only works on ActiveRecord objects, and before new examples it -reloads or recreates the model, _only_ if needed. That is, when you changed -properties or destroyed the object. - -There is one gotcha; you can't reference a model defined in a `let` block in a -`set` block. - -### Time-sensitive tests - -[Timecop](https://github.com/travisjeffery/timecop) is available in our -Ruby-based tests for verifying things that are time-sensitive. Any test that -exercises or verifies something time-sensitive should make use of Timecop to -prevent transient test failures. - -Example: - -```ruby -it 'is overdue' do - issue = build(:issue, due_date: Date.tomorrow) - - Timecop.freeze(3.days.from_now) do - expect(issue).to be_overdue - end -end -``` - -### System / Feature tests - -- Feature specs should be named `ROLE_ACTION_spec.rb`, such as - `user_changes_password_spec.rb`. -- Use only one `feature` block per feature spec file. -- Use scenario titles that describe the success and failure paths. -- Avoid scenario titles that add no information, such as "successfully". -- Avoid scenario titles that repeat the feature title. - -### Table-based / Parameterized tests - -This style of testing is used to exercise one piece of code with a comprehensive -range of inputs. By specifying the test case once, alongside a table of inputs -and the expected output for each, your tests can be made easier to read and more -compact. - -We use the [rspec-parameterized](https://github.com/tomykaira/rspec-parameterized) -gem. A short example, using the table syntax and checking Ruby equality for a -range of inputs, might look like this: - -```ruby -describe "#==" do - using RSpec::Parameterized::TableSyntax - - let(:project1) { create(:project) } - let(:project2) { create(:project) } - where(:a, :b, :result) do - 1 | 1 | true - 1 | 2 | false - true | true | true - true | false | false - project1 | project1 | true - project2 | project2 | true - project 1 | project2 | false - end - - with_them do - it { expect(a == b).to eq(result) } - - it 'is isomorphic' do - expect(b == a).to eq(result) - end - end -end -``` - -### Matchers - -Custom matchers should be created to clarify the intent and/or hide the -complexity of RSpec expectations.They should be placed under -`spec/support/matchers/`. Matchers can be placed in subfolder if they apply to -a certain type of specs only (e.g. features, requests etc.) but shouldn't be if -they apply to multiple type of specs. - -#### have_gitlab_http_status - -Prefer `have_gitlab_http_status` over `have_http_status` because the former -could also show the response body whenever the status mismatched. This would -be very useful whenever some tests start breaking and we would love to know -why without editing the source and rerun the tests. - -This is especially useful whenever it's showing 500 internal server error. - -### Shared contexts - -All shared contexts should be be placed under `spec/support/shared_contexts/`. -Shared contexts can be placed in subfolder if they apply to a certain type of -specs only (e.g. features, requests etc.) but shouldn't be if they apply to -multiple type of specs. - -Each file should include only one context and have a descriptive name, e.g. -`spec/support/shared_contexts/controllers/githubish_import_controller_shared_context.rb`. - -### Shared examples - -All shared examples should be be placed under `spec/support/shared_examples/`. -Shared examples can be placed in subfolder if they apply to a certain type of -specs only (e.g. features, requests etc.) but shouldn't be if they apply to -multiple type of specs. - -Each file should include only one context and have a descriptive name, e.g. -`spec/support/shared_examples/controllers/githubish_import_controller_shared_example.rb`. - -### Helpers - -Helpers are usually modules that provide some methods to hide the complexity of -specific RSpec examples. You can define helpers in RSpec files if they're not -intended to be shared with other specs. Otherwise, they should be be placed -under `spec/support/helpers/`. Helpers can be placed in subfolder if they apply -to a certain type of specs only (e.g. features, requests etc.) but shouldn't be -if they apply to multiple type of specs. - -Helpers should follow the Rails naming / namespacing convention. For instance -`spec/support/helpers/cycle_analytics_helpers.rb` should define: - -```ruby -module Spec - module Support - module Helpers - module CycleAnalyticsHelpers - def create_commit_referencing_issue(issue, branch_name: random_git_name) - project.repository.add_branch(user, branch_name, 'master') - create_commit("Commit for ##{issue.iid}", issue.project, user, branch_name) - end - end - end - end -end -``` - -Helpers should not change the RSpec config. For instance, the helpers module -described above should not include: - -```ruby -RSpec.configure do |config| - config.include Spec::Support::Helpers::CycleAnalyticsHelpers -end -``` - -### Factories - -GitLab uses [factory_girl] as a test fixture replacement. - -- Factory definitions live in `spec/factories/`, named using the pluralization - of their corresponding model (`User` factories are defined in `users.rb`). -- There should be only one top-level factory definition per file. -- FactoryGirl methods are mixed in to all RSpec groups. This means you can (and - should) call `create(...)` instead of `FactoryGirl.create(...)`. -- Make use of [traits] to clean up definitions and usages. -- When defining a factory, don't define attributes that are not required for the - resulting record to pass validation. -- When instantiating from a factory, don't supply attributes that aren't - required by the test. -- Factories don't have to be limited to `ActiveRecord` objects. - [See example](https://gitlab.com/gitlab-org/gitlab-ce/commit/0b8cefd3b2385a21cfed779bd659978c0402766d). - -[factory_girl]: https://github.com/thoughtbot/factory_girl -[traits]: http://www.rubydoc.info/gems/factory_girl/file/GETTING_STARTED.md#Traits - -### Fixtures - -All fixtures should be be placed under `spec/fixtures/`. - -### Config - -RSpec config files are files that change the RSpec config (i.e. -`RSpec.configure do |config|` blocks). They should be placed under -`spec/support/config/`. - -Each file should be related to a specific domain, e.g. -`spec/support/config/capybara.rb`, `spec/support/config/carrierwave.rb`, etc. - -Helpers can be included in the `spec/support/config/rspec.rb` file. If a -helpers module applies only to a certain kind of specs, it should add modifiers -to the `config.include` call. For instance if -`spec/support/helpers/cycle_analytics_helpers.rb` applies to `:lib` and -`type: :model` specs only, you would write the following: - -```ruby -RSpec.configure do |config| - config.include Spec::Support::Helpers::CycleAnalyticsHelpers, :lib - config.include Spec::Support::Helpers::CycleAnalyticsHelpers, type: :model -end -``` - -## Testing Rake Tasks - -To make testing Rake tasks a little easier, there is a helper that can be included -in lieu of the standard Spec helper. Instead of `require 'spec_helper'`, use -`require 'rake_helper'`. The helper includes `spec_helper` for you, and configures -a few other things to make testing Rake tasks easier. - -At a minimum, requiring the Rake helper will redirect `stdout`, include the -runtime task helpers, and include the `RakeHelpers` Spec support module. - -The `RakeHelpers` module exposes a `run_rake_task(<task>)` method to make -executing tasks simple. See `spec/support/rake_helpers.rb` for all available -methods. - -Example: - -```ruby -require 'rake_helper' - -describe 'gitlab:shell rake tasks' do - before do - Rake.application.rake_require 'tasks/gitlab/shell' - - stub_warn_user_is_not_gitlab - end - - describe 'install task' do - it 'invokes create_hooks task' do - expect(Rake::Task['gitlab:shell:create_hooks']).to receive(:invoke) - - run_rake_task('gitlab:shell:install') - end - end -end -``` - -## Test speed - -GitLab has a massive test suite that, without [parallelization], can take hours -to run. It's important that we make an effort to write tests that are accurate -and effective _as well as_ fast. - -Here are some things to keep in mind regarding test performance: - -- `double` and `spy` are faster than `FactoryGirl.build(...)` -- `FactoryGirl.build(...)` and `.build_stubbed` are faster than `.create`. -- Don't `create` an object when `build`, `build_stubbed`, `attributes_for`, - `spy`, or `double` will do. Database persistence is slow! -- Don't mark a feature as requiring JavaScript (through `@javascript` in - Spinach or `:js` in RSpec) unless it's _actually_ required for the test - to be valid. Headless browser testing is slow! - -[parallelization]: #test-suite-parallelization-on-the-ci - -### Test suite parallelization on the CI - -Our current CI parallelization setup is as follows: - -1. The `retrieve-tests-metadata` job in the `prepare` stage ensures that we have - a `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file: - - The `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file is fetched - from S3, if it's not here we initialize the file with `{}`. -1. Each `rspec-pg x y`/`rspec-mysql x y` job is run with `knapsack rspec` and - should have an evenly distributed share of tests: - - It works because the jobs have access to the - `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` since the "artifacts - from all previous stages are passed by default". [^1] - - The jobs set their own report path to - `KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json`. - - If knapsack is doing its job, test files that are run should be listed under - `Report specs`, not under `Leftover specs`. -1. The `update-tests-metadata` job takes all the - `knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json` - files from the `rspec-pg x y`/`rspec-mysql x y`jobs and merge them all together - into a single `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file that - is then uploaded to S3. - -After that, the next pipeline will use the up-to-date -`knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file. The same strategy -is used for Spinach tests as well. - -### Monitoring - -The GitLab test suite is [monitored] for the `master` branch, and any branch -that includes `rspec-profile` in their name. - -A [public dashboard] is available for everyone to see. Feel free to look at the -slowest test files and try to improve them. - -[monitored]: ./performance.md#rspec-profiling -[public dashboard]: https://redash.gitlab.com/public/dashboards/l1WhHXaxrCWM5Ai9D7YDqHKehq6OU3bx5gssaiWe?org_slug=default - -## CI setup - -- On CE and EE, the test suite runs both PostgreSQL and MySQL. -- Rails logging to `log/test.log` is disabled by default in CI [for - performance reasons][logging]. To override this setting, provide the - `RAILS_ENABLE_TEST_LOG` environment variable. - -[logging]: https://jtway.co/speed-up-your-rails-test-suite-by-6-in-1-line-13fedb869ec4 - -## Spinach (feature) tests - -GitLab [moved from Cucumber to Spinach](https://github.com/gitlabhq/gitlabhq/pull/1426) -for its feature/integration tests in September 2012. - -As of March 2016, we are [trying to avoid adding new Spinach -tests](https://gitlab.com/gitlab-org/gitlab-ce/issues/14121) going forward, -opting for [RSpec feature](#features-integration) specs. - -Adding new Spinach scenarios is acceptable _only if_ the new scenario requires -no more than one new `step` definition. If more than that is required, the -test should be re-implemented using RSpec instead. - ---- - -[Return to Development documentation](README.md) - -[^1]: /ci/yaml/README.html#dependencies +This document was moved to [testing_guide/index.md](testing_guide/index.md). diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md new file mode 100644 index 00000000000..613423dbd9a --- /dev/null +++ b/doc/development/testing_guide/best_practices.md @@ -0,0 +1,272 @@ +# Testing best practices + +## Test speed + +GitLab has a massive test suite that, without [parallelization], can take hours +to run. It's important that we make an effort to write tests that are accurate +and effective _as well as_ fast. + +Here are some things to keep in mind regarding test performance: + +- `double` and `spy` are faster than `FactoryGirl.build(...)` +- `FactoryGirl.build(...)` and `.build_stubbed` are faster than `.create`. +- Don't `create` an object when `build`, `build_stubbed`, `attributes_for`, + `spy`, or `double` will do. Database persistence is slow! +- Don't mark a feature as requiring JavaScript (through `@javascript` in + Spinach or `:js` in RSpec) unless it's _actually_ required for the test + to be valid. Headless browser testing is slow! + +[parallelization]: ci.md#test-suite-parallelization-on-the-ci + +## RSpec + +### General guidelines + +- Use a single, top-level `describe ClassName` block. +- Use `.method` to describe class methods and `#method` to describe instance + methods. +- Use `context` to test branching logic. +- Don't assert against the absolute value of a sequence-generated attribute (see [Gotchas](../gotchas.md#dont-assert-against-the-absolute-value-of-a-sequence-generated-attribute)). +- Try to match the ordering of tests to the ordering within the class. +- Try to follow the [Four-Phase Test][four-phase-test] pattern, using newlines + to separate phases. +- Use `Gitlab.config.gitlab.host` rather than hard coding `'localhost'` +- Don't assert against the absolute value of a sequence-generated attribute (see + [Gotchas](../gotchas.md#dont-assert-against-the-absolute-value-of-a-sequence-generated-attribute)). +- Don't supply the `:each` argument to hooks since it's the default. +- On `before` and `after` hooks, prefer it scoped to `:context` over `:all` + +[four-phase-test]: https://robots.thoughtbot.com/four-phase-test + +### System / Feature tests + +NOTE: **Note:** Before writing a new system test, [please consider **not** +writing one](testing_levels.md#consider-not-writing-a-system-test)! + +- Feature specs should be named `ROLE_ACTION_spec.rb`, such as + `user_changes_password_spec.rb`. +- Use scenario titles that describe the success and failure paths. +- Avoid scenario titles that add no information, such as "successfully". +- Avoid scenario titles that repeat the feature title. +- Create only the necessary records in the database +- Test a happy path and a less happy path but that's it +- Every other possible path should be tested with Unit or Integration tests +- Test what's displayed on the page, not the internals of ActiveRecord models. + For instance, if you want to verify that a record was created, add + expectations that its attributes are displayed on the page, not that + `Model.count` increased by one. +- It's ok to look for DOM elements but don't abuse it since it makes the tests + more brittle + +### `let` variables + +GitLab's RSpec suite has made extensive use of `let` variables to reduce +duplication. However, this sometimes [comes at the cost of clarity][lets-not], +so we need to set some guidelines for their use going forward: + +- `let` variables are preferable to instance variables. Local variables are + preferable to `let` variables. +- Use `let` to reduce duplication throughout an entire spec file. +- Don't use `let` to define variables used by a single test; define them as + local variables inside the test's `it` block. +- Don't define a `let` variable inside the top-level `describe` block that's + only used in a more deeply-nested `context` or `describe` block. Keep the + definition as close as possible to where it's used. +- Try to avoid overriding the definition of one `let` variable with another. +- Don't define a `let` variable that's only used by the definition of another. + Use a helper method instead. + +[lets-not]: https://robots.thoughtbot.com/lets-not + +### `set` variables + +In some cases there is no need to recreate the same object for tests again for +each example. For example, a project is needed to test issues on the same +project, one project will do for the entire file. This can be achieved by using +`set` in the same way you would use `let`. + +`rspec-set` only works on ActiveRecord objects, and before new examples it +reloads or recreates the model, _only_ if needed. That is, when you changed +properties or destroyed the object. + +There is one gotcha; you can't reference a model defined in a `let` block in a +`set` block. + +### Time-sensitive tests + +[Timecop](https://github.com/travisjeffery/timecop) is available in our +Ruby-based tests for verifying things that are time-sensitive. Any test that +exercises or verifies something time-sensitive should make use of Timecop to +prevent transient test failures. + +Example: + +```ruby +it 'is overdue' do + issue = build(:issue, due_date: Date.tomorrow) + + Timecop.freeze(3.days.from_now) do + expect(issue).to be_overdue + end +end +``` + +### Table-based / Parameterized tests + +This style of testing is used to exercise one piece of code with a comprehensive +range of inputs. By specifying the test case once, alongside a table of inputs +and the expected output for each, your tests can be made easier to read and more +compact. + +We use the [rspec-parameterized](https://github.com/tomykaira/rspec-parameterized) +gem. A short example, using the table syntax and checking Ruby equality for a +range of inputs, might look like this: + +```ruby +describe "#==" do + using RSpec::Parameterized::TableSyntax + + let(:project1) { create(:project) } + let(:project2) { create(:project) } + where(:a, :b, :result) do + 1 | 1 | true + 1 | 2 | false + true | true | true + true | false | false + project1 | project1 | true + project2 | project2 | true + project 1 | project2 | false + end + + with_them do + it { expect(a == b).to eq(result) } + + it 'is isomorphic' do + expect(b == a).to eq(result) + end + end +end +``` + +### Matchers + +Custom matchers should be created to clarify the intent and/or hide the +complexity of RSpec expectations.They should be placed under +`spec/support/matchers/`. Matchers can be placed in subfolder if they apply to +a certain type of specs only (e.g. features, requests etc.) but shouldn't be if +they apply to multiple type of specs. + +#### `have_gitlab_http_status` + +Prefer `have_gitlab_http_status` over `have_http_status` because the former +could also show the response body whenever the status mismatched. This would +be very useful whenever some tests start breaking and we would love to know +why without editing the source and rerun the tests. + +This is especially useful whenever it's showing 500 internal server error. + +### Shared contexts + +All shared contexts should be be placed under `spec/support/shared_contexts/`. +Shared contexts can be placed in subfolder if they apply to a certain type of +specs only (e.g. features, requests etc.) but shouldn't be if they apply to +multiple type of specs. + +Each file should include only one context and have a descriptive name, e.g. +`spec/support/shared_contexts/controllers/githubish_import_controller_shared_context.rb`. + +### Shared examples + +All shared examples should be be placed under `spec/support/shared_examples/`. +Shared examples can be placed in subfolder if they apply to a certain type of +specs only (e.g. features, requests etc.) but shouldn't be if they apply to +multiple type of specs. + +Each file should include only one context and have a descriptive name, e.g. +`spec/support/shared_examples/controllers/githubish_import_controller_shared_example.rb`. + +### Helpers + +Helpers are usually modules that provide some methods to hide the complexity of +specific RSpec examples. You can define helpers in RSpec files if they're not +intended to be shared with other specs. Otherwise, they should be be placed +under `spec/support/helpers/`. Helpers can be placed in subfolder if they apply +to a certain type of specs only (e.g. features, requests etc.) but shouldn't be +if they apply to multiple type of specs. + +Helpers should follow the Rails naming / namespacing convention. For instance +`spec/support/helpers/cycle_analytics_helpers.rb` should define: + +```ruby +module Spec + module Support + module Helpers + module CycleAnalyticsHelpers + def create_commit_referencing_issue(issue, branch_name: random_git_name) + project.repository.add_branch(user, branch_name, 'master') + create_commit("Commit for ##{issue.iid}", issue.project, user, branch_name) + end + end + end + end +end +``` + +Helpers should not change the RSpec config. For instance, the helpers module +described above should not include: + +```ruby +RSpec.configure do |config| + config.include Spec::Support::Helpers::CycleAnalyticsHelpers +end +``` + +### Factories + +GitLab uses [factory_girl] as a test fixture replacement. + +- Factory definitions live in `spec/factories/`, named using the pluralization + of their corresponding model (`User` factories are defined in `users.rb`). +- There should be only one top-level factory definition per file. +- FactoryGirl methods are mixed in to all RSpec groups. This means you can (and + should) call `create(...)` instead of `FactoryGirl.create(...)`. +- Make use of [traits] to clean up definitions and usages. +- When defining a factory, don't define attributes that are not required for the + resulting record to pass validation. +- When instantiating from a factory, don't supply attributes that aren't + required by the test. +- Factories don't have to be limited to `ActiveRecord` objects. + [See example](https://gitlab.com/gitlab-org/gitlab-ce/commit/0b8cefd3b2385a21cfed779bd659978c0402766d). + +[factory_girl]: https://github.com/thoughtbot/factory_girl +[traits]: http://www.rubydoc.info/gems/factory_girl/file/GETTING_STARTED.md#Traits + +### Fixtures + +All fixtures should be be placed under `spec/fixtures/`. + +### Config + +RSpec config files are files that change the RSpec config (i.e. +`RSpec.configure do |config|` blocks). They should be placed under +`spec/support/config/`. + +Each file should be related to a specific domain, e.g. +`spec/support/config/capybara.rb`, `spec/support/config/carrierwave.rb`, etc. + +Helpers can be included in the `spec/support/config/rspec.rb` file. If a +helpers module applies only to a certain kind of specs, it should add modifiers +to the `config.include` call. For instance if +`spec/support/helpers/cycle_analytics_helpers.rb` applies to `:lib` and +`type: :model` specs only, you would write the following: + +```ruby +RSpec.configure do |config| + config.include Spec::Support::Helpers::CycleAnalyticsHelpers, :lib + config.include Spec::Support::Helpers::CycleAnalyticsHelpers, type: :model +end +``` + +--- + +[Return to Testing documentation](index.md) diff --git a/doc/development/testing_guide/ci.md b/doc/development/testing_guide/ci.md new file mode 100644 index 00000000000..e90de55068d --- /dev/null +++ b/doc/development/testing_guide/ci.md @@ -0,0 +1,52 @@ +# GitLab tests in the Continuous Integration (CI) context + +### Test suite parallelization on the CI + +Our current CI parallelization setup is as follows: + +1. The `knapsack` job in the prepare stage that is supposed to ensure we have a + `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file: + - The `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file is fetched + from S3, if it's not here we initialize the file with `{}`. +1. Each `rspec x y` job are run with `knapsack rspec` and should have an evenly + distributed share of tests: + - It works because the jobs have access to the + `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` since the "artifacts + from all previous stages are passed by default". [^1] + - the jobs set their own report path to + `KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json`. + - if knapsack is doing its job, test files that are run should be listed under + `Report specs`, not under `Leftover specs`. +1. The `update-knapsack` job takes all the + `knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json` + files from the `rspec x y` jobs and merge them all together into a single + `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file that is then + uploaded to S3. + +After that, the next pipeline will use the up-to-date +`knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file. The same strategy +is used for Spinach tests as well. + +### Monitoring + +The GitLab test suite is [monitored] for the `master` branch, and any branch +that includes `rspec-profile` in their name. + +A [public dashboard] is available for everyone to see. Feel free to look at the +slowest test files and try to improve them. + +[monitored]: ../performance.md#rspec-profiling +[public dashboard]: https://redash.gitlab.com/public/dashboards/l1WhHXaxrCWM5Ai9D7YDqHKehq6OU3bx5gssaiWe?org_slug=default + +## CI setup + +- On CE and EE, the test suite runs both PostgreSQL and MySQL. +- Rails logging to `log/test.log` is disabled by default in CI [for + performance reasons][logging]. To override this setting, provide the + `RAILS_ENABLE_TEST_LOG` environment variable. + +[logging]: https://jtway.co/speed-up-your-rails-test-suite-by-6-in-1-line-13fedb869ec4 + +--- + +[Return to Testing documentation](index.md) diff --git a/doc/development/testing_guide/flaky_tests.md b/doc/development/testing_guide/flaky_tests.md new file mode 100644 index 00000000000..bbb2313ea7b --- /dev/null +++ b/doc/development/testing_guide/flaky_tests.md @@ -0,0 +1,74 @@ +# Flaky tests + +## What's a flaky test? + +It's a test that sometimes fails, but if you retry it enough times, it passes, +eventually. + +## Automatic retries and flaky tests detection + +On our CI, we use [rspec-retry] to automatically retry a failing example a few +times (see [`spec/spec_helper.rb`] for the precise retries count). + +We also use a home-made `RspecFlaky::Listener` listener which records flaky +examples in a JSON report file on `master` (`retrieve-tests-metadata` and `update-tests-metadata` jobs), and warns when a new flaky example +is detected in any other branch (`flaky-examples-check` job). In the future, the +`flaky-examples-check` job will not be allowed to fail. + +This was originally implemented in: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/13021. + +[rspec-retry]: https://github.com/NoRedInk/rspec-retry +[`spec/spec_helper.rb`]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/spec_helper.rb + +## Problems we had in the past at GitLab + +- [`rspec-retry` is bitting us when some API specs fail](https://gitlab.com/gitlab-org/gitlab-ce/issues/29242): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9825 +- [Sporadic RSpec failures due to `PG::UniqueViolation`](https://gitlab.com/gitlab-org/gitlab-ce/issues/28307#note_24958837): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9846 + - Follow-up: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10688 + - [Capybara.reset_session! should be called before requests are blocked](https://gitlab.com/gitlab-org/gitlab-ce/issues/33779): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12224 +- FFaker generates funky data that tests are not ready to handle (and tests should be predictable so that's bad!): + - [Make `spec/mailers/notify_spec.rb` more robust](https://gitlab.com/gitlab-org/gitlab-ce/issues/20121): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10015 + - [Transient failure in spec/requests/api/commits_spec.rb](https://gitlab.com/gitlab-org/gitlab-ce/issues/27988#note_25342521): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9944 + - [Replace FFaker factory data with sequences](https://gitlab.com/gitlab-org/gitlab-ce/issues/29643): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10184 + - [Transient failure in spec/finders/issues_finder_spec.rb](https://gitlab.com/gitlab-org/gitlab-ce/issues/30211#note_26707685): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10404 + +### Time-sensitive flaky tests + +- https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10046 +- https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10306 + +### Array order expectation + +- https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10148 + +### Feature tests + +- [Be sure to create all the data the test need before starting exercize](https://gitlab.com/gitlab-org/gitlab-ce/issues/32622#note_31128195): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12059 +- [Bis](https://gitlab.com/gitlab-org/gitlab-ce/issues/34609#note_34048715): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12604 +- [Bis](https://gitlab.com/gitlab-org/gitlab-ce/issues/34698#note_34276286): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12664 +- [Assert against the underlying database state instead of against a page's content](https://gitlab.com/gitlab-org/gitlab-ce/issues/31437): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10934 + +#### Capybara viewport size related issues + +- [Transient failure of spec/features/issues/filtered_search/filter_issues_spec.rb](https://gitlab.com/gitlab-org/gitlab-ce/issues/29241#note_26743936): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10411 + +#### Capybara JS driver related issues + +- [Don't wait for AJAX when no AJAX request is fired](https://gitlab.com/gitlab-org/gitlab-ce/issues/30461): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10454 +- [Bis](https://gitlab.com/gitlab-org/gitlab-ce/issues/34647): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12626 + +#### PhantomJS / WebKit related issues + +- Memory is through the roof! (TL;DR: Load images but block images requests!): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12003 + +## Resources + +- [Flaky Tests: Are You Sure You Want to Rerun Them?](http://semaphoreci.com/blog/2017/04/20/flaky-tests.html) +- [How to Deal With and Eliminate Flaky Tests](https://semaphoreci.com/community/tutorials/how-to-deal-with-and-eliminate-flaky-tests) +- [Tips on Treating Flakiness in your Rails Test Suite](http://semaphoreci.com/blog/2017/08/03/tips-on-treating-flakiness-in-your-test-suite.html) +- ['Flaky' tests: a short story](https://www.ombulabs.com/blog/rspec/continuous-integration/how-to-track-down-a-flaky-test.html) +- [Using Insights to Discover Flaky, Slow, and Failed Tests](https://circleci.com/blog/using-insights-to-discover-flaky-slow-and-failed-tests/) + +--- + +[Return to Testing documentation](index.md) diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md new file mode 100644 index 00000000000..0c63f51cb45 --- /dev/null +++ b/doc/development/testing_guide/frontend_testing.md @@ -0,0 +1,254 @@ +# Frontend testing standards and style guidelines + +There are two types of test suites you'll encounter while developing frontend code +at GitLab. We use Karma and Jasmine for JavaScript unit and integration testing, +and RSpec feature tests with Capybara for e2e (end-to-end) integration testing. + +Unit and feature tests need to be written for all new features. +Most of the time, you should use [RSpec] for your feature tests. + +Regression tests should be written for bug fixes to prevent them from recurring +in the future. + +See the [Testing Standards and Style Guidelines](index.md) page for more +information on general testing practices at GitLab. + +## Karma test suite + +GitLab uses the [Karma][karma] test runner with [Jasmine] as its test +framework for our JavaScript unit and integration tests. For integration tests, +we generate HTML files using RSpec (see `spec/javascripts/fixtures/*.rb` for examples). +Some fixtures are still HAML templates that are translated to HTML files using the same mechanism (see `static_fixtures.rb`). +Adding these static fixtures should be avoided as they are harder to keep up to date with real views. +The existing static fixtures will be migrated over time. +Please see [gitlab-org/gitlab-ce#24753](https://gitlab.com/gitlab-org/gitlab-ce/issues/24753) to track our progress. +Fixtures are served during testing by the [jasmine-jquery][jasmine-jquery] plugin. + +JavaScript tests live in `spec/javascripts/`, matching the folder structure +of `app/assets/javascripts/`: `app/assets/javascripts/behaviors/autosize.js` +has a corresponding `spec/javascripts/behaviors/autosize_spec.js` file. + +Keep in mind that in a CI environment, these tests are run in a headless +browser and you will not have access to certain APIs, such as +[`Notification`](https://developer.mozilla.org/en-US/docs/Web/API/notification), +which will have to be stubbed. + +### Best practices + +#### Naming unit tests + +When writing describe test blocks to test specific functions/methods, +please use the method name as the describe block name. + +```javascript +// Good +describe('methodName', () => { + it('passes', () => { + expect(true).toEqual(true); + }); +}); + +// Bad +describe('#methodName', () => { + it('passes', () => { + expect(true).toEqual(true); + }); +}); + +// Bad +describe('.methodName', () => { + it('passes', () => { + expect(true).toEqual(true); + }); +}); +``` +#### Testing promises + +When testing Promises you should always make sure that the test is asynchronous and rejections are handled. +Your Promise chain should therefore end with a call of the `done` callback and `done.fail` in case an error occurred. + +```javascript +// Good +it('tests a promise', (done) => { + promise + .then((data) => { + expect(data).toBe(asExpected); + }) + .then(done) + .catch(done.fail); +}); + +// Good +it('tests a promise rejection', (done) => { + promise + .then(done.fail) + .catch((error) => { + expect(error).toBe(expectedError); + }) + .then(done) + .catch(done.fail); +}); + +// Bad (missing done callback) +it('tests a promise', () => { + promise + .then((data) => { + expect(data).toBe(asExpected); + }) +}); + +// Bad (missing catch) +it('tests a promise', (done) => { + promise + .then((data) => { + expect(data).toBe(asExpected); + }) + .then(done) +}); + +// Bad (use done.fail in asynchronous tests) +it('tests a promise', (done) => { + promise + .then((data) => { + expect(data).toBe(asExpected); + }) + .then(done) + .catch(fail) +}); + +// Bad (missing catch) +it('tests a promise rejection', (done) => { + promise + .catch((error) => { + expect(error).toBe(expectedError); + }) + .then(done) +}); +``` + +#### Stubbing + +For unit tests, you should stub methods that are unrelated to the current unit you are testing. +If you need to use a prototype method, instantiate an instance of the class and call it there instead of mocking the instance completely. + +For integration tests, you should stub methods that will effect the stability of the test if they +execute their original behaviour. i.e. Network requests. + +### Vue.js unit tests + +See this [section][vue-test]. + +### Running frontend tests + +`rake karma` runs the frontend-only (JavaScript) tests. +It consists of two subtasks: + +- `rake karma:fixtures` (re-)generates fixtures +- `rake karma:tests` actually executes the tests + +As long as the fixtures don't change, `rake karma:tests` (or `yarn karma`) +is sufficient (and saves you some time). + +### Live testing and focused testing + +While developing locally, it may be helpful to keep karma running so that you +can get instant feedback on as you write tests and modify code. To do this +you can start karma with `npm run karma-start`. It will compile the javascript +assets and run a server at `http://localhost:9876/` where it will automatically +run the tests on any browser which connects to it. You can enter that url on +multiple browsers at once to have it run the tests on each in parallel. + +While karma is running, any changes you make will instantly trigger a recompile +and retest of the entire test suite, so you can see instantly if you've broken +a test with your changes. You can use [jasmine focused][jasmine-focus] or +excluded tests (with `fdescribe` or `xdescribe`) to get karma to run only the +tests you want while you're working on a specific feature, but make sure to +remove these directives when you commit your code. + +## RSpec feature integration tests + +Information on setting up and running RSpec integration tests with +[Capybara] can be found in the [Testing Best Practices](best_practices.md). + +## Gotchas + +### Errors due to use of unsupported JavaScript features + +Similar errors will be thrown if you're using JavaScript features not yet +supported by the PhantomJS test runner which is used for both Karma and RSpec +tests. We polyfill some JavaScript objects for older browsers, but some +features are still unavailable: + +- Array.from +- Array.first +- Async functions +- Generators +- Array destructuring +- For..Of +- Symbol/Symbol.iterator +- Spread + +Until these are polyfilled appropriately, they should not be used. Please +update this list with additional unsupported features. + +### RSpec errors due to JavaScript + +By default RSpec unit tests will not run JavaScript in the headless browser +and will simply rely on inspecting the HTML generated by rails. + +If an integration test depends on JavaScript to run correctly, you need to make +sure the spec is configured to enable JavaScript when the tests are run. If you +don't do this you'll see vague error messages from the spec runner. + +To enable a JavaScript driver in an `rspec` test, add `:js` to the +individual spec or the context block containing multiple specs that need +JavaScript enabled: + +```ruby +# For one spec +it 'presents information about abuse report', :js do + # assertions... +end + +describe "Admin::AbuseReports", :js do + it 'presents information about abuse report' do + # assertions... + end + it 'shows buttons for adding to abuse report' do + # assertions... + end +end +``` + +### Spinach errors due to missing JavaScript + +NOTE: **Note:** Since we are discouraging the use of Spinach when writing new +feature tests, you shouldn't ever need to use this. This information is kept +available for legacy purposes only. + +In Spinach, the JavaScript driver is enabled differently. In the `*.feature` +file for the failing spec, add the `@javascript` flag above the Scenario: + +``` +@javascript +Scenario: Developer can approve merge request + Given I am a "Shop" developer + And I visit project "Shop" merge requests page + And merge request 'Bug NS-04' must be approved + And I click link "Bug NS-04" + When I click link "Approve" + Then I should see approved merge request "Bug NS-04" +``` + +[jasmine-focus]: https://jasmine.github.io/2.5/focused_specs.html +[jasmine-jquery]: https://github.com/velesin/jasmine-jquery +[karma]: http://karma-runner.github.io/ +[vue-test]:https://docs.gitlab.com/ce/development/fe_guide/vue.html#testing-vue-components +[RSpec]: https://github.com/rspec/rspec-rails#feature-specs +[Capybara]: https://github.com/teamcapybara/capybara +[Karma]: http://karma-runner.github.io/ +[Jasmine]: https://jasmine.github.io/ + +--- + +[Return to Testing documentation](index.md) diff --git a/doc/development/fe_guide/img/testing_triangle.png b/doc/development/testing_guide/img/testing_triangle.png Binary files differindex 7a9a848c2ee..7a9a848c2ee 100644 --- a/doc/development/fe_guide/img/testing_triangle.png +++ b/doc/development/testing_guide/img/testing_triangle.png diff --git a/doc/development/testing_guide/index.md b/doc/development/testing_guide/index.md new file mode 100644 index 00000000000..38b1fe1a193 --- /dev/null +++ b/doc/development/testing_guide/index.md @@ -0,0 +1,90 @@ +# Testing standards and style guidelines + +This document describes various guidelines and best practices for automated +testing of the GitLab project. + +It is meant to be an _extension_ of the [thoughtbot testing +styleguide](https://github.com/thoughtbot/guides/tree/master/style/testing). If +this guide defines a rule that contradicts the thoughtbot guide, this guide +takes precedence. Some guidelines may be repeated verbatim to stress their +importance. + +## Overview + +GitLab is built on top of [Ruby on Rails][rails], and we're using [RSpec] for all +the backend tests, with [Capybara] for end-to-end integration testing. +On the frontend side, we're using [Karma] and [Jasmine] for JavaScript unit and +integration testing. + +Following are two great articles that everyone should read to understand what +automated testing means, and what are its principles: + +- [Five Factor Testing](https://www.devmynd.com/blog/five-factor-testing): Why do we need tests? +- [Principles of Automated Testing](http://www.lihaoyi.com/post/PrinciplesofAutomatedTesting.html): Levels of testing. Prioritize tests. Cost of tests. + +--- + +## [Testing levels](testing_levels.md) + +Learn about the different testing levels, and how to decide at what level your +changes should be tested. + +--- + +## [Testing best practices](best_practices.md) + +Everything you should know about how to write good tests: RSpec, FactoryGirl, +system tests, parameterized tests etc. + +--- + +## [Frontend testing standards and style guidelines](frontend_testing.md) + +Everything you should know about how to write good Frontend tests: Karma, +testing promises, stubbing etc. + +--- + +## [Flaky tests](flaky_tests.md) + +What are flaky tests, the different kind of flaky tests we encountered, and what +we do about them. + +--- + +## [GitLab tests in the Continuous Integration (CI) context](ci.md) + +How GitLab test suite is run in the CI context: setup, caches, artifacts, +parallelization, monitoring. + +--- + +## [Testing Rake tasks](testing_rake_tasks.md) + +Everything you should know about how to test Rake tasks. + +--- + +## Spinach (feature) tests + +GitLab [moved from Cucumber to Spinach](https://github.com/gitlabhq/gitlabhq/pull/1426) +for its feature/integration tests in September 2012. + +As of March 2016, we are [trying to avoid adding new Spinach +tests](https://gitlab.com/gitlab-org/gitlab-ce/issues/14121) going forward, +opting for [RSpec feature](#features-integration) specs. + +Adding new Spinach scenarios is acceptable _only if_ the new scenario requires +no more than one new `step` definition. If more than that is required, the +test should be re-implemented using RSpec instead. + +--- + +[Return to Development documentation](../README.md) + +[^1]: /ci/yaml/README.html#dependencies + +[RSpec]: https://github.com/rspec/rspec-rails#feature-specs +[Capybara]: https://github.com/teamcapybara/capybara +[Karma]: http://karma-runner.github.io/ +[Jasmine]: https://jasmine.github.io/ diff --git a/doc/development/testing_guide/testing_levels.md b/doc/development/testing_guide/testing_levels.md new file mode 100644 index 00000000000..9b9ba0baa71 --- /dev/null +++ b/doc/development/testing_guide/testing_levels.md @@ -0,0 +1,173 @@ +# Testing levels + + + +_This diagram demonstrates the relative priority of each test type we use. `e2e` stands for end-to-end._ + +## Unit tests + +Formal definition: https://en.wikipedia.org/wiki/Unit_testing + +These kind of tests ensure that a single unit of code (a method) works as +expected (given an input, it has a predictable output). These tests should be +isolated as much as possible. For example, model methods that don't do anything +with the database shouldn't need a DB record. Classes that don't need database +records should use stubs/doubles as much as possible. + +| Code path | Tests path | Testing engine | Notes | +| --------- | ---------- | -------------- | ----- | +| `app/finders/` | `spec/finders/` | RSpec | | +| `app/helpers/` | `spec/helpers/` | RSpec | | +| `app/db/{post_,}migrate/` | `spec/migrations/` | RSpec | More details at [`spec/migrations/README.md`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/migrations/README.md). | +| `app/policies/` | `spec/policies/` | RSpec | | +| `app/presenters/` | `spec/presenters/` | RSpec | | +| `app/routing/` | `spec/routing/` | RSpec | | +| `app/serializers/` | `spec/serializers/` | RSpec | | +| `app/services/` | `spec/services/` | RSpec | | +| `app/tasks/` | `spec/tasks/` | RSpec | | +| `app/uploaders/` | `spec/uploaders/` | RSpec | | +| `app/views/` | `spec/views/` | RSpec | | +| `app/workers/` | `spec/workers/` | RSpec | | +| `app/assets/javascripts/` | `spec/javascripts/` | Karma | More details in the [Frontent Testing guide](frontend_testing.md) section. | + +## Integration tests + +Formal definition: https://en.wikipedia.org/wiki/Integration_testing + +These kind of tests ensure that individual parts of the application work well together, without the overhead of the actual app environment (i.e. the browser). These tests should assert at the request/response level: status code, headers, body. They're useful to test permissions, redirections, what view is rendered etc. + +| Code path | Tests path | Testing engine | Notes | +| --------- | ---------- | -------------- | ----- | +| `app/controllers/` | `spec/controllers/` | RSpec | | +| `app/mailers/` | `spec/mailers/` | RSpec | | +| `lib/api/` | `spec/requests/api/` | RSpec | | +| `lib/ci/api/` | `spec/requests/ci/api/` | RSpec | | +| `app/assets/javascripts/` | `spec/javascripts/` | Karma | More details in the [JavaScript](#javascript) section. | + +### About controller tests + +In an ideal world, controllers should be thin. However, when this is not the +case, it's acceptable to write a system/feature test without JavaScript instead +of a controller test. The reason is that testing a fat controller usually +involves a lot of stubbing, things like: + +```ruby +controller.instance_variable_set(:@user, user) +``` + +and use methods which are deprecated in Rails 5 ([#23768]). + +[#23768]: https://gitlab.com/gitlab-org/gitlab-ce/issues/23768 + +### About Karma + +As you may have noticed, Karma is both in the Unit tests and the Integration +tests category. That's because Karma is a tool that provides an environment to +run JavaScript tests, so you can either run unit tests (e.g. test a single +JavaScript method), or integration tests (e.g. test a component that is composed +of multiple components). + +## System tests or feature tests + +Formal definition: https://en.wikipedia.org/wiki/System_testing. + +These kind of tests ensure the application works as expected from a user point +of view (aka black-box testing). These tests should test a happy path for a +given page or set of pages, and a test case should be added for any regression +that couldn't have been caught at lower levels with better tests (i.e. if a +regression is found, regression tests should be added at the lowest-level +possible). + +| Tests path | Testing engine | Notes | +| ---------- | -------------- | ----- | +| `spec/features/` | [Capybara] + [RSpec] | If your spec has the `:js` metadata, the browser driver will be [Poltergeist], otherwise it's using [RackTest]. | +| `features/` | Spinach | Spinach tests are deprecated, [you shouldn't add new Spinach tests](#spinach-feature-tests). | + +### Consider **not** writing a system test! + +If we're confident that the low-level components work well (and we should be if +we have enough Unit & Integration tests), we shouldn't need to duplicate their +thorough testing at the System test level. + +It's very easy to add tests, but a lot harder to remove or improve tests, so one +should take care of not introducing too many (slow and duplicated) specs. + +The reasons why we should follow these best practices are as follows: + +- System tests are slow to run since they spin up the entire application stack + in a headless browser, and even slower when they integrate a JS driver +- When system tests run with a JavaScript driver, the tests are run in a + different thread than the application. This means it does not share a + database connection and your test will have to commit the transactions in + order for the running application to see the data (and vice-versa). In that + case we need to truncate the database after each spec instead of simply + rolling back a transaction (the faster strategy that's in use for other kind + of tests). This is slower than transactions, however, so we want to use + truncation only when necessary. + +[Poltergeist]: https://github.com/teamcapybara/capybara#poltergeist +[RackTest]: https://github.com/teamcapybara/capybara#racktest + +## Black-box tests or end-to-end tests + +GitLab consists of [multiple pieces] such as [GitLab Shell], [GitLab Workhorse], +[Gitaly], [GitLab Pages], [GitLab Runner], and GitLab Rails. All theses pieces +are configured and packaged by [GitLab Omnibus]. + +[GitLab QA] is a tool that allows to test that all these pieces integrate well +together by building a Docker image for a given version of GitLab Rails and +running feature tests (i.e. using Capybara) against it. + +The actual test scenarios and steps are [part of GitLab Rails] so that they're +always in-sync with the codebase. + +[multiple pieces]: ../architecture.md#components +[GitLab Shell]: https://gitlab.com/gitlab-org/gitlab-shell +[GitLab Workhorse]: https://gitlab.com/gitlab-org/gitlab-workhorse +[Gitaly]: https://gitlab.com/gitlab-org/gitaly +[GitLab Pages]: https://gitlab.com/gitlab-org/gitlab-pages +[GitLab Runner]: https://gitlab.com/gitlab-org/gitlab-ci-multi-runner +[GitLab Omnibus]: https://gitlab.com/gitlab-org/omnibus-gitlab +[GitLab QA]: https://gitlab.com/gitlab-org/gitlab-qa +[part of GitLab Rails]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa + +## How to test at the correct level? + +As many things in life, deciding what to test at each level of testing is a +trade-off: + +- Unit tests are usually cheap, and you should consider them like the basement + of your house: you need them to be confident that your code is behaving + correctly. However if you run only unit tests without integration / system + tests, you might [miss] the [big] [picture]! +- Integration tests are a bit more expensive, but don't abuse them. A system test + is often better than an integration test that is stubbing a lot of internals. +- System tests are expensive (compared to unit tests), even more if they require + a JavaScript driver. Make sure to follow the guidelines in the [Speed](#test-speed) + section. + +Another way to see it is to think about the "cost of tests", this is well +explained [in this article][tests-cost] and the basic idea is that the cost of a +test includes: + +- The time it takes to write the test +- The time it takes to run the test every time the suite runs +- The time it takes to understand the test +- The time it takes to fix the test if it breaks and the underlying code is OK +- Maybe, the time it takes to change the code to make the code testable. + +### Frontend-related tests + +There are cases where the behaviour you are testing is not worth the time spent +running the full application, for example, if you are testing styling, animation, +edge cases or small actions that don't involve the backend, +you should write an integration test using Jasmine. + +[miss]: https://twitter.com/ThePracticalDev/status/850748070698651649 +[big]: https://twitter.com/timbray/status/822470746773409794 +[picture]: https://twitter.com/withzombies/status/829716565834752000 +[tests-cost]: https://medium.com/table-xi/high-cost-tests-and-high-value-tests-a86e27a54df#.2ulyh3a4e + +--- + +[Return to Testing documentation](index.md) diff --git a/doc/development/testing_guide/testing_rake_tasks.md b/doc/development/testing_guide/testing_rake_tasks.md new file mode 100644 index 00000000000..5bf185dd7b5 --- /dev/null +++ b/doc/development/testing_guide/testing_rake_tasks.md @@ -0,0 +1,39 @@ +## Testing Rake tasks + +To make testing Rake tasks a little easier, there is a helper that can be included +in lieu of the standard Spec helper. Instead of `require 'spec_helper'`, use +`require 'rake_helper'`. The helper includes `spec_helper` for you, and configures +a few other things to make testing Rake tasks easier. + +At a minimum, requiring the Rake helper will redirect `stdout`, include the +runtime task helpers, and include the `RakeHelpers` Spec support module. + +The `RakeHelpers` module exposes a `run_rake_task(<task>)` method to make +executing tasks simple. See `spec/support/rake_helpers.rb` for all available +methods. + +Example: + +```ruby +require 'rake_helper' + +describe 'gitlab:shell rake tasks' do + before do + Rake.application.rake_require 'tasks/gitlab/shell' + + stub_warn_user_is_not_gitlab + end + + describe 'install task' do + it 'invokes create_hooks task' do + expect(Rake::Task['gitlab:shell:create_hooks']).to receive(:invoke) + + run_rake_task('gitlab:shell:install') + end + end +end +``` + +--- + +[Return to Testing documentation](index.md) diff --git a/doc/install/kubernetes/gitlab_chart.md b/doc/install/kubernetes/gitlab_chart.md index ddfd47df099..fa564d83785 100644 --- a/doc/install/kubernetes/gitlab_chart.md +++ b/doc/install/kubernetes/gitlab_chart.md @@ -1,6 +1,6 @@ # GitLab Helm Chart > **Note**: -* This chart will be replaced by the [gitlab-omnibus](gitlab_omnibus.md) chart, once it supports [additional configuration options](https://gitlab.com/charts/charts.gitlab.io/issues/68). For more information on available charts, please see our [overview](index.md#chart-overview). +* This chart is deprecated, and is being replaced by the [cloud native GitLab chart](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md). For more information on available charts, please see our [overview](index.md#chart-overview). * These charts have been tested on Google Container Engine and Azure Container Service. Other Kubernetes installations may work as well, if not please [open an issue](https://gitlab.com/charts/charts.gitlab.io/issues). @@ -8,7 +8,9 @@ For more information on available GitLab Helm Charts, please see our [overview]( ## Introduction -The `gitlab` Helm chart deploys just GitLab into your Kubernetes cluster, and offers extensive configuration options. This chart requires advanced knowledge of Kubernetes to successfully use. For most deployments we **strongly recommended** the [gitlab-omnibus](gitlab_omnibus.md) chart, which will replace this chart once it supports [additional configuration options](https://gitlab.com/charts/charts.gitlab.io/issues/68). Due to the difficulty in supporting upgrades to the `omnibus-gitlab` chart, migrating will require exporting data out of this instance and importing it into the new deployment. +The `gitlab` Helm chart deploys just GitLab into your Kubernetes cluster, and offers extensive configuration options. This chart requires advanced knowledge of Kubernetes to successfully use. We **strongly recommend** the [gitlab-omnibus](gitlab_omnibus.md) chart. + +This chart is deprecated, and will be replaced by the [cloud native GitLab chart](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md). Due to the difficulty in supporting upgrades, migrating will require exporting data out of this instance and importing it into the new deployment. This chart includes the following: diff --git a/doc/install/kubernetes/gitlab_omnibus.md b/doc/install/kubernetes/gitlab_omnibus.md index 8c110a37380..6659c3cf7b2 100644 --- a/doc/install/kubernetes/gitlab_omnibus.md +++ b/doc/install/kubernetes/gitlab_omnibus.md @@ -1,7 +1,6 @@ # GitLab-Omnibus Helm Chart > **Note:** -* This Helm chart is in beta, while [additional features](https://gitlab.com/charts/charts.gitlab.io/issues/68) are being worked on. -* GitLab is working on a [cloud native set of Charts](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) which will eventually replace these. +* This Helm chart is in beta, and will be deprecated by the [cloud native GitLab chart](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md). * These charts have been tested on Google Container Engine and Azure Container Service. Other Kubernetes installations may work as well, if not please [open an issue](https://gitlab.com/charts/charts.gitlab.io/issues). This work is based partially on: https://github.com/lwolf/kubernetes-gitlab/. GitLab would like to thank Sergey Nuzhdin for his work. @@ -12,6 +11,8 @@ For more information on available GitLab Helm Charts, please see our [overview]( This chart provides an easy way to get started with GitLab, provisioning an installation with nearly all functionality enabled. SSL is automatically provisioned via [Let's Encrypt](https://letsencrypt.org/). +This Helm chart is in beta, and will be deprecated by the [cloud native GitLab chart](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) once available. Due to the difficulty in supporting upgrades, migrating will require exporting data out of this instance and importing it into the new deployment. + The deployment includes: - A [GitLab Omnibus](https://docs.gitlab.com/omnibus/) Pod, including Mattermost, Container Registry, and Prometheus @@ -23,9 +24,8 @@ The deployment includes: ### Limitations -* This chart is suited for small to medium size deployments, because [High Availability](https://docs.gitlab.com/ee/administration/high_availability/) and [Geo](https://docs.gitlab.com/ee/gitlab-geo/README.html) will not be supported. -* It is in beta. Additional features to support production deployments, like backups, are [in development](https://gitlab.com/charts/charts.gitlab.io/issues/68). Once completed, this chart will be generally available. -* A new generation of [cloud native charts](index.md#upcoming-cloud-native-helm-charts) is in development, and will eventually deprecate these. Due to the difficulty in supporting upgrades to the new architecture, migrating will require exporting data out of this instance and importing it into the new deployment. We do not expect these to be production ready before the second half of 2018. +* This chart is in beta, and suited for small to medium size deployments. [High Availability](https://docs.gitlab.com/ee/administration/high_availability/) and [Geo](https://docs.gitlab.com/ee/gitlab-geo/README.html) are not supported. +* A new generation [cloud native GitLab chart](index.md#cloud-native-gitlab-chart) is in development, and will deprecate this chart. Due to the difficulty in supporting upgrades to the new architecture, migrating will require exporting data out of this instance and importing it into the new deployment. We plan to release the new chart in beta by the end of 2017. For more information on available GitLab Helm Charts, please see our [overview](index.md#chart-overview). diff --git a/doc/install/kubernetes/index.md b/doc/install/kubernetes/index.md index aed00ae9e2c..dd350820c18 100644 --- a/doc/install/kubernetes/index.md +++ b/doc/install/kubernetes/index.md @@ -9,11 +9,11 @@ should be deployed, upgraded, and configured. ## Chart Overview -* **[GitLab-Omnibus](gitlab_omnibus.md)**: The best way to run GitLab on Kubernetes today. It is suited for small to medium deployments, and is in beta while support for backups and other features are added. +* **[GitLab-Omnibus](gitlab_omnibus.md)**: The best way to run GitLab on Kubernetes today, suited for small to medium deployments. The chart is in beta and will be deprecated by the [cloud native GitLab chart](#cloud-native-gitlab-chart). * **[Cloud Native GitLab Chart](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md)**: The next generation GitLab chart, currently in development. Will support large deployments with horizontal scaling of individual GitLab components. * Other Charts * [GitLab Runner Chart](gitlab_runner_chart.md): For deploying just the GitLab Runner. - * [Advanced GitLab Installation](gitlab_chart.md): Provides additional deployment options, but provides less functionality out-of-the-box. It's beta, no longer actively developed, and will be deprecated by [gitlab-omnibus](#gitlab-omnibus-chart-recommended) once it supports these options. + * [Advanced GitLab Installation](gitlab_chart.md): Deprecated, being replaced by the [cloud native GitLab chart](#cloud-native-gitlab-chart). Provides additional deployment options, but provides less functionality out-of-the-box. * [Community Contributed Charts](#community-contributed-charts): Community contributed charts, deprecated by the official GitLab chart. ## GitLab-Omnibus Chart (Recommended) @@ -21,13 +21,13 @@ should be deployed, upgraded, and configured. This chart is the best available way to operate GitLab on Kubernetes. It deploys and configures nearly all features of GitLab, including: a [Runner](https://docs.gitlab.com/runner/), [Container Registry](../../user/project/container_registry.html#gitlab-container-registry), [Mattermost](https://docs.gitlab.com/omnibus/gitlab-mattermost/), [automatic SSL](https://github.com/kubernetes/charts/tree/master/stable/kube-lego), and a [load balancer](https://github.com/kubernetes/ingress/tree/master/controllers/nginx). It is based on our [GitLab Omnibus Docker Images](https://docs.gitlab.com/omnibus/docker/README.html). -Once the [cloud native charts](#upcoming-cloud-native-helm-charts) are ready for production use, this chart will be deprecated. Due to the difficulty in supporting upgrades to the new architecture, migrating will require exporting data out of this instance and importing it into the new deployment. +Once the [cloud native GitLab chart](#cloud-native-gitlab-chart) is ready for production use, this chart will be deprecated. Due to the difficulty in supporting upgrades to the new architecture, migrating will require exporting data out of this instance and importing it into the new deployment. -Learn more about the [gitlab-omnibus chart.](gitlab_omnibus.md) +Learn more about the [gitlab-omnibus chart](gitlab_omnibus.md). ## Cloud Native GitLab Chart -GitLab is working towards building a [cloud native GitLab chart](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md). A key part of this effort is to isolate each service into its [own Docker container and Helm chart](https://gitlab.com/gitlab-org/omnibus-gitlab/issues/2420), rather than utilizing the all-in-one container image of the [current charts](#official-gitlab-helm-charts-recommended). +GitLab is working towards building a [cloud native GitLab chart](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md). A key part of this effort is to isolate each service into its [own Docker container and Helm chart](https://gitlab.com/gitlab-org/omnibus-gitlab/issues/2420), rather than utilizing the all-in-one container image of the [current chart](#gitlab-omnibus-chart-recommended). By offering individual containers and charts, we will be able to provide a number of benefits: * Easier horizontal scaling of each service, @@ -35,9 +35,9 @@ By offering individual containers and charts, we will be able to provide a numbe * Potential for rolling updates and canaries within a service, * and plenty more. -This is a large project and will be worked on over the span of multiple releases. For the most up-to-date status and release information, please see our [tracking issue](https://gitlab.com/gitlab-org/omnibus-gitlab/issues/2420). We do not expect these to be production ready before the second half of 2018. +This is a large project and will be worked on over the span of multiple releases. For the most up-to-date status and release information, please see our [tracking issue](https://gitlab.com/gitlab-org/omnibus-gitlab/issues/2420). We are planning to launch this chart in beta by the end of 2017. -Learn more about the [cloud native GitLab chart.](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) +Learn more about the [cloud native GitLab chart](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md). ## Other Charts diff --git a/doc/user/discussions/img/image_resolved_discussion.png b/doc/user/discussions/img/image_resolved_discussion.png Binary files differnew file mode 100755 index 00000000000..ed00b5c77fe --- /dev/null +++ b/doc/user/discussions/img/image_resolved_discussion.png diff --git a/doc/user/discussions/img/onion_skin_view.png b/doc/user/discussions/img/onion_skin_view.png Binary files differnew file mode 100755 index 00000000000..91c3b396844 --- /dev/null +++ b/doc/user/discussions/img/onion_skin_view.png diff --git a/doc/user/discussions/img/start_image_discussion.gif b/doc/user/discussions/img/start_image_discussion.gif Binary files differnew file mode 100644 index 00000000000..43efbf2fbb2 --- /dev/null +++ b/doc/user/discussions/img/start_image_discussion.gif diff --git a/doc/user/discussions/img/swipe_view.png b/doc/user/discussions/img/swipe_view.png Binary files differnew file mode 100755 index 00000000000..82d6e52173c --- /dev/null +++ b/doc/user/discussions/img/swipe_view.png diff --git a/doc/user/discussions/img/two_up_view.png b/doc/user/discussions/img/two_up_view.png Binary files differnew file mode 100755 index 00000000000..d9e90708e87 --- /dev/null +++ b/doc/user/discussions/img/two_up_view.png diff --git a/doc/user/discussions/index.md b/doc/user/discussions/index.md index ab46befb91c..5bd326a426f 100644 --- a/doc/user/discussions/index.md +++ b/doc/user/discussions/index.md @@ -153,42 +153,71 @@ comments in greater detail.  -## Locking discussions +## Image discussions > [Introduced][ce-14531] in GitLab 10.1. -There might be some cases where a discussion is better off if it's locked down. -For example: +Sometimes a discussion is revolved around an image. With image discussions, +you can easily target a specific coordinate of an image and start a discussion +around it. Image discussions are available in merge requests and commit detail views. -- Discussions that are several years old and the issue/merge request is closed, - but people continue to try to resurrect the discussion. -- Discussions where someone or a group of people are trolling, are abusive, or - in-general are causing the discussion to be unproductive. +To start an image discussion, hover your mouse over the image. Your mouse pointer +should convert into an icon, indicating that the image is available for commenting. +Simply click anywhere on the image to create a new discussion. -In locked discussions, only team members can write new comments and edit the old -ones. + -To lock or unlock a discussion, you need to have at least Master [permissions]: +After you click on the image, a comment form will be displayed that would be the start +of your discussion. Once you save your comment, you will see a new badge displayed on +top of your image. This badge represents your discussion. -1. Find the "Lock" section in the sidebar and click **Edit** -1. In the dialog that will appear, you can choose to turn on or turn off the - discussion lock -1. Optionally, leave a comment to explain your reasoning behind that action +>**Note:** +This discussion badge is typically associated with a number that is only used as a visual +reference for each discussion. In the merge request discussion tab, +this badge will be indicated with a comment icon since each discussion will render a new +image section. -| Turn off discussion lock | Turn on discussion lock | +Image discussions also work on diffs that replace an existing image. In this diff view +mode, you can toggle the different view modes and still see the discussion point badges. + +| 2-up | Swipe | Onion Skin | +| :-----------: | :----------: | :----------: | +|  |  |  | + +Image discussions also work well with resolvable discussions. Resolved discussions +on diffs (not on the merge request discussion tab) will appear collapsed on page +load and will have a corresponding badge counter to match the counter on the image. + + + +## Lock discussions + +> [Introduced][ce-14531] in GitLab 10.1. + +For large projects with many contributors, it may be useful to stop discussions +in issues or merge requests in these scenarios: + +- The project maintainer has already resolved the discussion and it is not helpful +for continued feedback. The project maintainer has already directed new conversation +to newer issues or merge requests. +- The people participating in the discussion are trolling, abusive, or otherwise +being unproductive. + +In these cases, a user with Master permissions or higher in the project can lock (and unlock) +an issue or a merge request, using the "Lock" section in the sidebar: + +| Unlock | Lock | | :-----------: | :----------: | |  |  | -Every change is indicated by a system note in the issue's or merge request's -comments. +System notes indicate locking and unlocking.  -Once an issue or merge request is locked, project members can see the indicator -in the comment area, whereas non project members can only see the information -that the discussion is locked. +In a locked issue or merge request, only team members can add new comments and +edit existing comments. Non-team members are restricted from adding or editing comments. -| Team member | Not a member | +| Team member | Non-team member | | :-----------: | :----------: | |  |  | diff --git a/doc/user/permissions.md b/doc/user/permissions.md index be72d42625a..c4e41c8e9bf 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -25,7 +25,7 @@ The following table depicts the various user permission levels in a project. | Create confidential issue | ✓ [^1] | ✓ | ✓ | ✓ | ✓ | | View confidential issues | (✓) [^2] | ✓ | ✓ | ✓ | ✓ | | Leave comments | ✓ [^1] | ✓ | ✓ | ✓ | ✓ | -| Lock comments | | | | ✓ | ✓ | +| Lock discussions (issues and merge requests) | | | | ✓ | ✓ | | See a list of jobs | ✓ [^3] | ✓ | ✓ | ✓ | ✓ | | See a job log | ✓ [^3] | ✓ | ✓ | ✓ | ✓ | | Download and browse job artifacts | ✓ [^3] | ✓ | ✓ | ✓ | ✓ | @@ -54,7 +54,7 @@ The following table depicts the various user permission levels in a project. | Create or update commit status | | | ✓ | ✓ | ✓ | | Update a container registry | | | ✓ | ✓ | ✓ | | Remove a container registry image | | | ✓ | ✓ | ✓ | -| Create new milestones | | | | ✓ | ✓ | +| Create/edit/delete project milestones | | | ✓ | ✓ | ✓ | | Add new team members | | | | ✓ | ✓ | | Push to protected branches | | | | ✓ | ✓ | | Enable/disable branch protection | | | | ✓ | ✓ | @@ -143,6 +143,7 @@ group. | Manage group members | | | | | ✓ | | Remove group | | | | | ✓ | | Manage group labels | | ✓ | ✓ | ✓ | ✓ | +| Create/edit/delete group milestones | | | ✓ | ✓ | ✓ | ### Subgroup permissions diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index e68160c8faf..7c02c9c5c48 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -33,7 +33,6 @@ module Gitlab explore favicon.ico files - google_api groups health_check help diff --git a/lib/gitlab/project_template.rb b/lib/gitlab/project_template.rb index 732fbf68dad..0b43377a579 100644 --- a/lib/gitlab/project_template.rb +++ b/lib/gitlab/project_template.rb @@ -1,9 +1,9 @@ module Gitlab class ProjectTemplate - attr_reader :title, :name + attr_reader :title, :name, :description, :preview - def initialize(name, title) - @name, @title = name, title + def initialize(name, title, description, preview) + @name, @title, @description, @preview = name, title, description, preview end alias_method :logo, :name @@ -25,9 +25,9 @@ module Gitlab end TEMPLATES_TABLE = [ - ProjectTemplate.new('rails', 'Ruby on Rails'), - ProjectTemplate.new('spring', 'Spring'), - ProjectTemplate.new('express', 'NodeJS Express') + ProjectTemplate.new('rails', 'Ruby on Rails', 'Includes a MVC structure, gemfile, rakefile, and .gitlab-ci.yml file, along with many others, to help you get started.', 'https://gitlab.com/gitlab-org/project-templates/rails'), + ProjectTemplate.new('spring', 'Spring', 'Includes a MVC structure, mvnw, pom.xml, and .gitlab-ci.yml file to help you get started.', 'https://gitlab.com/gitlab-org/project-templates/spring'), + ProjectTemplate.new('express', 'NodeJS Express', 'Includes a MVC structure, and .gitlab-ci.yml file to help you get started.', 'https://gitlab.com/gitlab-org/project-templates/express') ].freeze class << self diff --git a/qa/qa/runtime/namespace.rb b/qa/qa/runtime/namespace.rb index 996286430b9..b00e925986b 100644 --- a/qa/qa/runtime/namespace.rb +++ b/qa/qa/runtime/namespace.rb @@ -8,7 +8,7 @@ module QA end def name - 'qa_test_' + time.strftime('%d_%m_%Y_%H-%M-%S') + 'qa-test-' + time.strftime('%d-%m-%Y-%H-%M-%S') end def sandbox_name diff --git a/spec/features/merge_requests/diff_notes_resolve_spec.rb b/spec/features/merge_requests/diff_notes_resolve_spec.rb index 637e6036384..475c8586f45 100644 --- a/spec/features/merge_requests/diff_notes_resolve_spec.rb +++ b/spec/features/merge_requests/diff_notes_resolve_spec.rb @@ -88,14 +88,24 @@ feature 'Diff notes resolve', :js do end end - it 'hides resolved discussion' do - page.within '.diff-content' do - click_button 'Resolve discussion' + describe 'resolved discussion' do + before do + page.within '.diff-content' do + click_button 'Resolve discussion' + end + + visit_merge_request end - visit_merge_request + it 'hides when resolve discussion is clicked' do + expect(page).to have_selector('.discussion-body', visible: false) + end - expect(page).to have_selector('.discussion-body', visible: false) + it 'shows resolved discussion when toggled' do + find(".timeline-content .discussion[data-discussion-id='#{note.discussion_id}'] .discussion-toggle-button").click + + expect(page.find(".timeline-content #note_#{note.noteable_id}")).to be_visible + end end it 'allows user to resolve from reply form without a comment' do diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb index c928459f911..026aa03f7cf 100644 --- a/spec/features/projects/import_export/import_file_spec.rb +++ b/spec/features/projects/import_export/import_file_spec.rb @@ -27,6 +27,7 @@ feature 'Import/Export - project import integration test', :js do select2(namespace.id, from: '#project_namespace_id') fill_in :project_path, with: project_path, visible: true + click_import_project_tab click_link 'GitLab export' expect(page).to have_content('Import an exported GitLab project') @@ -51,6 +52,7 @@ feature 'Import/Export - project import integration test', :js do context 'path is not prefilled' do scenario 'user imports an exported project successfully' do visit new_project_path + click_import_project_tab click_link 'GitLab export' fill_in :path, with: 'test-project-path', visible: true @@ -72,6 +74,7 @@ feature 'Import/Export - project import integration test', :js do select2(user.namespace.id, from: '#project_namespace_id') fill_in :project_path, with: project.name, visible: true + click_import_project_tab click_link 'GitLab export' attach_file('file', file) click_on 'Import project' @@ -81,19 +84,6 @@ feature 'Import/Export - project import integration test', :js do end end - context 'when limited to the default user namespace' do - scenario 'passes correct namespace ID in the URL' do - visit new_project_path - - fill_in :project_path, with: 'test-project-path', visible: true - - click_link 'GitLab export' - - expect(page).to have_content('GitLab project export') - expect(URI.parse(current_url).query).to eq("namespace_id=#{user.namespace.id}&path=test-project-path") - end - end - def wiki_exists?(project) wiki = ProjectWiki.new(project) File.exist?(wiki.repository.path_to_repo) && !wiki.repository.empty? @@ -102,4 +92,8 @@ feature 'Import/Export - project import integration test', :js do def project_hook_exists?(project) Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository).exists? end + + def click_import_project_tab + find('#import-project-tab').trigger('click') + end end diff --git a/spec/features/projects/new_project_spec.rb b/spec/features/projects/new_project_spec.rb index cd3dc72d3c6..8e11cb94350 100644 --- a/spec/features/projects/new_project_spec.rb +++ b/spec/features/projects/new_project_spec.rb @@ -9,12 +9,14 @@ feature 'New project' do sign_in(user) end - it 'shows "New project" page' do + it 'shows "New project" page', :js do visit new_project_path expect(page).to have_content('Project path') expect(page).to have_content('Project name') + find('#import-project-tab').trigger('click') + expect(page).to have_link('GitHub') expect(page).to have_link('Bitbucket') expect(page).to have_link('GitLab.com') @@ -23,14 +25,15 @@ feature 'New project' do expect(page).to have_link('GitLab export') end - context 'Visibility level selector' do + context 'Visibility level selector', :js do Gitlab::VisibilityLevel.options.each do |key, level| it "sets selector to #{key}" do stub_application_setting(default_project_visibility: level) visit new_project_path - - expect(find_field("project_visibility_level_#{level}")).to be_checked + page.within('#blank-project-pane') do + expect(find_field("project_visibility_level_#{level}")).to be_checked + end end it "saves visibility level #{level} on validation error" do @@ -38,8 +41,9 @@ feature 'New project' do choose(s_(key)) click_button('Create project') - - expect(find_field("project_visibility_level_#{level}")).to be_checked + page.within('#blank-project-pane') do + expect(find_field("project_visibility_level_#{level}")).to be_checked + end end end end @@ -51,9 +55,11 @@ feature 'New project' do end it 'selects the user namespace' do - namespace = find('#project_namespace_id') + page.within('#blank-project-pane') do + namespace = find('#project_namespace_id') - expect(namespace.text).to eq user.username + expect(namespace.text).to eq user.username + end end end @@ -66,9 +72,11 @@ feature 'New project' do end it 'selects the group namespace' do - namespace = find('#project_namespace_id option[selected]') + page.within('#blank-project-pane') do + namespace = find('#project_namespace_id option[selected]') - expect(namespace.text).to eq group.name + expect(namespace.text).to eq group.name + end end end @@ -82,9 +90,11 @@ feature 'New project' do end it 'selects the group namespace' do - namespace = find('#project_namespace_id option[selected]') + page.within('#blank-project-pane') do + namespace = find('#project_namespace_id option[selected]') - expect(namespace.text).to eq subgroup.full_path + expect(namespace.text).to eq subgroup.full_path + end end end @@ -124,9 +134,10 @@ feature 'New project' do end end - context 'Import project options' do + context 'Import project options', :js do before do visit new_project_path + find('#import-project-tab').trigger('click') end context 'from git repository url' do diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index ac62280e4ca..3bc7ec3123f 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -12,8 +12,9 @@ feature 'Project' do visit new_project_path end - it "allows creation from templates" do - page.choose(template.name) + it "allows creation from templates", :js do + find('#create-from-template-tab').trigger('click') + find("##{template.name}").trigger('click') fill_in("project_path", with: template.name) page.within '#content-body' do diff --git a/spec/lib/gitlab/project_template_spec.rb b/spec/lib/gitlab/project_template_spec.rb index d19bd611919..57b0ef8d1ad 100644 --- a/spec/lib/gitlab/project_template_spec.rb +++ b/spec/lib/gitlab/project_template_spec.rb @@ -4,9 +4,9 @@ describe Gitlab::ProjectTemplate do describe '.all' do it 'returns a all templates' do expected = [ - described_class.new('rails', 'Ruby on Rails'), - described_class.new('spring', 'Spring'), - described_class.new('express', 'NodeJS Express') + described_class.new('rails', 'Ruby on Rails', 'Includes an MVC structure, .gitignore, Gemfile, and more great stuff', 'https://gitlab.com/gitlab-org/project-templates/rails'), + described_class.new('spring', 'Spring', 'Includes an MVC structure, .gitignore, Gemfile, and more great stuff', 'https://gitlab.com/gitlab-org/project-templates/spring'), + described_class.new('express', 'NodeJS Express', 'Includes an MVC structure, .gitignore, Gemfile, and more great stuff', 'https://gitlab.com/gitlab-org/project-templates/express') ] expect(described_class.all).to be_an(Array) @@ -31,7 +31,7 @@ describe Gitlab::ProjectTemplate do end describe 'instance methods' do - subject { described_class.new('phoenix', 'Phoenix Framework') } + subject { described_class.new('phoenix', 'Phoenix Framework', 'Phoenix description', 'link-to-template') } it { is_expected.to respond_to(:logo, :file, :archive_path) } end |
