diff options
| author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-06-23 10:44:03 +0200 |
|---|---|---|
| committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-06-23 10:44:03 +0200 |
| commit | f40b99d02ee9411e5a7f9a93e3e6cf33c1d7890e (patch) | |
| tree | a55ff4f3ceaa24fcb008114f62b8ac377db6a129 | |
| parent | 9369adb93d119ca349add2b7f80d6a9b4bbd6703 (diff) | |
| parent | 4aa1fdd347d1df4001d9e1298e6dc09c0c478c2e (diff) | |
| download | gitlab-ce-f40b99d02ee9411e5a7f9a93e3e6cf33c1d7890e.tar.gz | |
Merge branch 'master' into rubocop-for-tests
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Conflicts:
spec/features/issues_spec.rb
spec/models/forked_project_link_spec.rb
spec/models/hooks/service_hook_spec.rb
spec/models/hooks/web_hook_spec.rb
spec/models/project_services/hipchat_service_spec.rb
spec/requests/api/project_members_spec.rb
spec/requests/api/projects_spec.rb
spec/requests/api/system_hooks_spec.rb
spec/services/archive_repository_service_spec.rb
spec/support/matchers.rb
spec/tasks/gitlab/backup_rake_spec.rb
122 files changed, 735 insertions, 616 deletions
diff --git a/CHANGELOG b/CHANGELOG index 455a4339420..53258e7fd1a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -9,6 +9,7 @@ v 7.13.0 (unreleased) - Allow users to customize their default Dashboard page. - Update ssl_ciphers in Nginx example to remove DHE settings. This will deny forward secrecy for Android 2.3.7, Java 6 and OpenSSL 0.9.8 - Convert CRLF newlines to LF when committing using the web editor. + - API request /projects/:project_id/merge_requests?state=closed will return only closed merge requests without merged one. If you need ones that were merged - use state=merged. v 7.12.0 (unreleased) - Fix Error 500 when one user attempts to access a personal, internal snippet (Stan Hu) @@ -51,8 +52,8 @@ v 7.12.0 (unreleased) - Add validation to wiki page creation (only [a-zA-Z0-9/_-] are allowed) (Jeroen van Baarsen) - Fix new/empty milestones showing 100% completion value (Jonah Bishop) - Add a note when an Issue or Merge Request's title changes - - Consistently refer to MRs as either Accepted or Rejected. - - Add Accepted and Rejected tabs to MR lists. + - Consistently refer to MRs as either Merged or Closed. + - Add Merged tab to MR lists. - Prefix EmailsOnPush email subject with `[Git]`. - Group project contributions by both name and email. - Clarify navigation labels for Project Settings and Group Settings. @@ -222,16 +222,16 @@ group :development do end group :development, :test do + gem 'awesome_print' + gem 'byebug' + gem 'pry-rails' + gem 'coveralls', require: false + gem 'database_cleaner', '~> 1.4.0' + gem 'factory_girl_rails' + gem 'rspec-rails', '~> 3.3.0' gem 'rubocop', '0.28.0', require: false gem 'spinach-rails' - gem "rspec-rails", '2.99' - gem 'capybara', '~> 2.2.1' - gem 'capybara-screenshot', '~> 1.0.0' - gem "pry-rails" - gem "awesome_print" - gem "database_cleaner" - gem 'factory_girl_rails' # Prevent occasions where minitest is not bundled in packaged versions of ruby (see #3826) gem 'minitest', '~> 5.3.0' @@ -239,8 +239,9 @@ group :development, :test do # Generate Fake data gem 'ffaker', '~> 2.0.0' - # PhantomJS driver for Capybara - gem 'poltergeist', '~> 1.5.1' + gem 'capybara', '~> 2.3.0' + gem 'capybara-screenshot', '~> 1.0.0' + gem 'poltergeist', '~> 1.6.0' gem 'teaspoon', '~> 1.0.0' gem 'teaspoon-jasmine' @@ -249,14 +250,12 @@ group :development, :test do gem 'spring-commands-rspec', '~> 1.0.0' gem 'spring-commands-spinach', '~> 1.0.0' gem 'spring-commands-teaspoon', '~> 0.0.2' - - gem "byebug" end group :test do gem 'simplecov', require: false gem 'shoulda-matchers', '~> 2.8.0', require: false - gem 'email_spec' + gem 'email_spec', '~> 1.6.0' gem 'webmock', '~> 1.21.0' gem 'test_after_commit' end diff --git a/Gemfile.lock b/Gemfile.lock index e64a32b6230..b719dd4ab06 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -82,7 +82,7 @@ GEM columnize (~> 0.8) debugger-linecache (~> 1.2) cal-heatmap-rails (0.0.1) - capybara (2.2.1) + capybara (2.3.0) mime-types (>= 1.16) nokogiri (>= 1.3.3) rack (>= 1.0.0) @@ -125,7 +125,7 @@ GEM d3_rails (3.5.5) railties (>= 3.1.0) daemons (1.1.9) - database_cleaner (1.3.0) + database_cleaner (1.4.1) debug_inspector (0.0.2) debugger-linecache (1.2.0) default_value_for (3.0.0) @@ -154,7 +154,7 @@ GEM dotenv (0.9.0) dropzonejs-rails (0.4.14) rails (> 3.1) - email_spec (1.5.0) + email_spec (1.6.0) launchy (~> 2.1) mail (~> 2.2) encryptor (1.3.0) @@ -348,7 +348,7 @@ GEM actionpack (>= 3.0.0) activesupport (>= 3.0.0) kgio (2.9.2) - launchy (2.4.2) + launchy (2.4.3) addressable (~> 2.3) letter_opener (1.1.2) launchy (~> 2.2) @@ -431,8 +431,8 @@ GEM orm_adapter (0.5.0) parser (2.2.0.2) ast (>= 1.1, < 3.0) - pg (0.15.1) - poltergeist (1.5.1) + pg (0.18.2) + poltergeist (1.6.0) capybara (~> 2.1) cliver (~> 0.3.1) multi_json (~> 1.0) @@ -449,7 +449,7 @@ GEM quiet_assets (1.0.2) railties (>= 3.1, < 5.0) racc (1.4.10) - rack (1.5.4) + rack (1.5.5) rack-accept (0.4.5) rack (>= 0.4) rack-attack (4.3.0) @@ -530,21 +530,23 @@ GEM rqrcode (0.4.2) rqrcode-rails3 (0.1.7) rqrcode (>= 0.4.2) - rspec-collection_matchers (1.1.2) - rspec-expectations (>= 2.99.0.beta1) - rspec-core (2.99.2) - rspec-expectations (2.99.2) - diff-lcs (>= 1.1.3, < 2.0) - rspec-mocks (2.99.3) - rspec-rails (2.99.0) - actionpack (>= 3.0) - activemodel (>= 3.0) - activesupport (>= 3.0) - railties (>= 3.0) - rspec-collection_matchers - rspec-core (~> 2.99.0) - rspec-expectations (~> 2.99.0) - rspec-mocks (~> 2.99.0) + rspec-core (3.3.1) + rspec-support (~> 3.3.0) + rspec-expectations (3.3.0) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.3.0) + rspec-mocks (3.3.0) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.3.0) + rspec-rails (3.3.2) + actionpack (>= 3.0, < 4.3) + activesupport (>= 3.0, < 4.3) + railties (>= 3.0, < 4.3) + rspec-core (~> 3.3.0) + rspec-expectations (~> 3.3.0) + rspec-mocks (~> 3.3.0) + rspec-support (~> 3.3.0) + rspec-support (3.3.0) rubocop (0.28.0) astrolabe (~> 1.3) parser (>= 2.2.0.pre.7, < 3.0) @@ -707,7 +709,9 @@ GEM webmock (1.21.0) addressable (>= 2.3.6) crack (>= 0.3.2) - websocket-driver (0.3.3) + websocket-driver (0.5.4) + websocket-extensions (>= 0.1.0) + websocket-extensions (0.1.2) wikicloth (0.8.1) builder expression_parser @@ -735,7 +739,7 @@ DEPENDENCIES browser (~> 0.8.0) byebug cal-heatmap-rails (~> 0.0.1) - capybara (~> 2.2.1) + capybara (~> 2.3.0) capybara-screenshot (~> 1.0.0) carrierwave charlock_holmes @@ -744,7 +748,7 @@ DEPENDENCIES coveralls creole (~> 0.3.6) d3_rails (~> 3.5.5) - database_cleaner + database_cleaner (~> 1.4.0) default_value_for (~> 3.0.0) devise (= 3.2.4) devise-async (= 0.9.0) @@ -752,7 +756,7 @@ DEPENDENCIES diffy (~> 3.0.3) doorkeeper (= 2.1.3) dropzonejs-rails - email_spec + email_spec (~> 1.6.0) enumerize factory_girl_rails ffaker (~> 2.0.0) @@ -800,7 +804,7 @@ DEPENDENCIES omniauth-twitter org-ruby (= 0.9.12) pg - poltergeist (~> 1.5.1) + poltergeist (~> 1.6.0) pry-rails quiet_assets (~> 1.0.1) rack-attack (~> 4.3.0) @@ -815,7 +819,7 @@ DEPENDENCIES request_store rerun (~> 0.10.0) rqrcode-rails3 - rspec-rails (= 2.99) + rspec-rails (~> 3.3.0) rubocop (= 0.28.0) rugments (~> 1.0.0.beta7) sanitize (~> 2.0) diff --git a/README.md b/README.md index 85ea5c876af..336196a623d 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ The source of GitLab Community Edition is [hosted on GitLab.com](https://gitlab.com/gitlab-org/gitlab-ce/) and there are mirrors to make [contributing](CONTRIBUTING.md) as easy as possible. -#  GitLab +#  GitLab ## Open source software to collaborate on code @@ -101,4 +101,4 @@ Please see [Getting help for GitLab](https://about.gitlab.com/getting-help/) on ## Is it awesome? Thanks for [asking this question](https://twitter.com/supersloth/status/489462789384056832) Joshua. -[These people](https://twitter.com/gitlab/favorites) seem to like it.
\ No newline at end of file +[These people](https://twitter.com/gitlab/favorites) seem to like it. diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index 17ddde68f93..d2f0c43929f 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -1,7 +1,7 @@ class DashboardController < Dashboard::ApplicationController - before_action :load_projects, except: [:projects] + before_action :load_projects before_action :event_filter, only: :show - + respond_to :html def show diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 0bed2115dc7..2eccc0ee31f 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -45,10 +45,10 @@ class IssuableFinder def group return @group if defined?(@group) - @group = + @group = if params[:group_id].present? Group.find(params[:group_id]) - else + else nil end end @@ -56,10 +56,10 @@ class IssuableFinder def project return @project if defined?(@project) - @project = + @project = if params[:project_id].present? Project.find(params[:project_id]) - else + else nil end end @@ -76,7 +76,7 @@ class IssuableFinder return @milestones if defined?(@milestones) @milestones = - if milestones? && params[:milestone_title] != NONE + if milestones? && params[:milestone_title] != NONE Milestone.where(title: params[:milestone_title]) else nil @@ -90,7 +90,7 @@ class IssuableFinder def assignee return @assignee if defined?(@assignee) - @assignee = + @assignee = if assignee? && params[:assignee_id] != NONE User.find(params[:assignee_id]) else @@ -105,7 +105,7 @@ class IssuableFinder def author return @author if defined?(@author) - @author = + @author = if author? && params[:author_id] != NONE User.find(params[:author_id]) else @@ -148,8 +148,6 @@ class IssuableFinder case params[:state] when 'closed' items.closed - when 'rejected' - items.respond_to?(:rejected) ? items.rejected : items.closed when 'merged' items.respond_to?(:merged) ? items.merged : items.closed when 'all' diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 9889c995c74..0b46db4b1c3 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -281,10 +281,9 @@ module ApplicationHelper def state_filters_text_for(entity, project) titles = { - opened: "Open", - merged: "Accepted" + opened: "Open" } - + entity_title = titles[entity] || entity.to_s.humanize count = diff --git a/app/helpers/broadcast_messages_helper.rb b/app/helpers/broadcast_messages_helper.rb index 29ff47663da..6484dca6b55 100644 --- a/app/helpers/broadcast_messages_helper.rb +++ b/app/helpers/broadcast_messages_helper.rb @@ -1,9 +1,16 @@ module BroadcastMessagesHelper def broadcast_styling(broadcast_message) - if(broadcast_message.color || broadcast_message.font) - "background-color:#{broadcast_message.color};color:#{broadcast_message.font}" - else - "" + styling = '' + + if broadcast_message.color.present? + styling << "background-color: #{broadcast_message.color}" + styling << '; ' if broadcast_message.font.present? end + + if broadcast_message.font.present? + styling << "color: #{broadcast_message.font}" + end + + styling end end diff --git a/app/helpers/icons_helper.rb b/app/helpers/icons_helper.rb index a730684f8f3..30b17a736a7 100644 --- a/app/helpers/icons_helper.rb +++ b/app/helpers/icons_helper.rb @@ -1,4 +1,6 @@ module IconsHelper + include FontAwesome::Rails::IconHelper + # Creates an icon tag given icon name(s) and possible icon modifiers. # # Right now this method simply delegates directly to `fa_icon` from the diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index f771fe761ef..2f8e64c375f 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,4 +1,6 @@ module NotificationsHelper + include IconsHelper + def notification_icon(notification) if notification.disabled? icon('volume-off', class: 'ns-mute') diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index 10c39cb1ece..56849f28ff0 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -75,7 +75,7 @@ module Mentionable refs.reject! { |ref| without.include?(ref) } refs.each do |ref| - Note.create_cross_reference_note(ref, local_reference, a) + SystemNoteService.cross_reference(ref, local_reference, a) end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 487d62e65b6..7ecdaf6b2e0 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -125,16 +125,14 @@ class MergeRequest < ActiveRecord::Base validate :validate_fork scope :of_group, ->(group) { where("source_project_id in (:group_project_ids) OR target_project_id in (:group_project_ids)", group_project_ids: group.project_ids) } - scope :merged, -> { with_state(:merged) } scope :by_branch, ->(branch_name) { where("(source_branch LIKE :branch) OR (target_branch LIKE :branch)", branch: branch_name) } scope :cared, ->(user) { where('assignee_id = :user OR author_id = :user', user: user.id) } scope :by_milestone, ->(milestone) { where(milestone_id: milestone) } scope :in_projects, ->(project_ids) { where("source_project_id in (:project_ids) OR target_project_id in (:project_ids)", project_ids: project_ids) } scope :of_projects, ->(ids) { where(target_project_id: ids) } - # Closed scope for merge request should return - # both merged and closed mr's - scope :closed, -> { with_states(:closed, :merged) } - scope :rejected, -> { with_states(:closed) } + scope :merged, -> { with_state(:merged) } + scope :closed, -> { with_state(:closed) } + scope :closed_and_merged, -> { with_states(:closed, :merged) } def self.reference_prefix '!' @@ -417,4 +415,14 @@ class MergeRequest < ActiveRecord::Base def can_be_merged_by?(user) ::Gitlab::GitAccess.new(user, project).can_push_to_branch?(target_branch) end + + def state_human_name + if merged? + "Merged" + elsif closed? + "Closed" + else + "Open" + end + end end diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 9c543b37023..e0c5fec97b7 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -56,7 +56,7 @@ class Milestone < ActiveRecord::Base end def closed_items_count - self.issues.closed.count + self.merge_requests.closed.count + self.issues.closed.count + self.merge_requests.closed_and_merged.count end def total_items_count diff --git a/app/models/note.rb b/app/models/note.rb index 6a74d62b715..68b9d433ae0 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -63,11 +63,6 @@ class Note < ActiveRecord::Base after_update :set_references class << self - # TODO (rspeicher): Update usages - def create_cross_reference_note(*args) - SystemNoteService.cross_reference(*args) - end - def discussions_from_notes(notes) discussion_ids = [] discussions = [] diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 68d3b915fc9..6135ae65007 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -105,7 +105,7 @@ class GitPushService author ||= commit_user(commit) refs.each do |r| - Note.create_cross_reference_note(r, commit, author) + SystemNoteService.cross_reference(r, commit, author) end end end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 0ff37c41743..482c0444049 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -15,7 +15,7 @@ module Notes # Create a cross-reference note if this Note contains GFM that names an # issue, merge request, or commit. note.references.each do |mentioned| - Note.create_cross_reference_note(mentioned, note.noteable, note.author) + SystemNoteService.cross_reference(mentioned, note.noteable, note.author) end execute_hooks(note) diff --git a/app/services/notes/update_service.rb b/app/services/notes/update_service.rb index 45a0db761ec..b5611d46257 100644 --- a/app/services/notes/update_service.rb +++ b/app/services/notes/update_service.rb @@ -13,7 +13,7 @@ module Notes # Create a cross-reference note if this Note contains GFM that # names an issue, merge request, or commit. note.references.each do |mentioned| - Note.create_cross_reference_note(mentioned, note.noteable, note.author) + SystemNoteService.cross_reference(mentioned, note.noteable, note.author) end end end diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml index d79c4548247..0bcd543fee7 100644 --- a/app/views/projects/merge_requests/_merge_request.html.haml +++ b/app/views/projects/merge_requests/_merge_request.html.haml @@ -9,11 +9,11 @@ - if merge_request.merged? %span %i.fa.fa-check - ACCEPTED + MERGED - elsif merge_request.closed? %span %i.fa.fa-ban - REJECTED + CLOSED - else %span.hidden-xs.hidden-sm %span.label-branch< diff --git a/app/views/projects/merge_requests/show/_mr_title.html.haml b/app/views/projects/merge_requests/show/_mr_title.html.haml index 0690fdb769f..83baf157a92 100644 --- a/app/views/projects/merge_requests/show/_mr_title.html.haml +++ b/app/views/projects/merge_requests/show/_mr_title.html.haml @@ -1,11 +1,6 @@ %h4.page-title .issue-box{ class: issue_box_class(@merge_request) } - - if @merge_request.merged? - Accepted - - elsif @merge_request.closed? - Rejected - - else - Open + = @merge_request.state_human_name = "Merge Request ##{@merge_request.iid}" %small.creator · diff --git a/app/views/projects/merge_requests/widget/_closed.html.haml b/app/views/projects/merge_requests/widget/_closed.html.haml index 18164ba771f..b5704c502c8 100644 --- a/app/views/projects/merge_requests/widget/_closed.html.haml +++ b/app/views/projects/merge_requests/widget/_closed.html.haml @@ -2,7 +2,7 @@ = render 'projects/merge_requests/widget/heading' .mr-widget-body %h4 - Rejected + Closed - if @merge_request.closed_event by #{link_to_member(@project, @merge_request.closed_event.author, avatar: true)} #{time_ago_with_tooltip(@merge_request.closed_event.created_at)} diff --git a/app/views/projects/merge_requests/widget/_merged.html.haml b/app/views/projects/merge_requests/widget/_merged.html.haml index 17c3fdacda8..a3b13140810 100644 --- a/app/views/projects/merge_requests/widget/_merged.html.haml +++ b/app/views/projects/merge_requests/widget/_merged.html.haml @@ -2,7 +2,7 @@ = render 'projects/merge_requests/widget/heading' .mr-widget-body %h4 - Accepted + Merged - if @merge_request.merge_event by #{link_to_member(@project, @merge_request.merge_event.author, avatar: true)} #{time_ago_with_tooltip(@merge_request.merge_event.created_at)} diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml index 417eaa1b09d..5c85092a045 100644 --- a/app/views/projects/milestones/show.html.haml +++ b/app/views/projects/milestones/show.html.haml @@ -86,10 +86,10 @@ .col-md-3 = render('merge_requests', title: 'Waiting for merge (open and assigned)', merge_requests: @merge_requests.opened.assigned, id: 'ongoing') .col-md-3 - = render('merge_requests', title: 'Rejected (closed)', merge_requests: @merge_requests.rejected, id: 'closed') + = render('merge_requests', title: 'Rejected (closed)', merge_requests: @merge_requests.closed, id: 'closed') .col-md-3 .panel.panel-primary - .panel-heading Accepted + .panel-heading Merged %ul.well-list - @merge_requests.merged.each do |merge_request| = render 'merge_request', merge_request: merge_request diff --git a/app/views/search/results/_merge_request.html.haml b/app/views/search/results/_merge_request.html.haml index adfdd1c7506..2efa616d664 100644 --- a/app/views/search/results/_merge_request.html.haml +++ b/app/views/search/results/_merge_request.html.haml @@ -11,6 +11,6 @@ #{merge_request.project.name_with_namespace} .pull-right - if merge_request.merged? - %span.label.label-primary Accepted + %span.label.label-primary Merged - elsif merge_request.closed? - %span.label.label-danger Rejected + %span.label.label-danger Closed diff --git a/app/views/shared/_issuable_filter.html.haml b/app/views/shared/_issuable_filter.html.haml index a5187fa4ea7..a355eb62813 100644 --- a/app/views/shared/_issuable_filter.html.haml +++ b/app/views/shared/_issuable_filter.html.haml @@ -12,10 +12,10 @@ = icon('check-circle') #{state_filters_text_for(:merged, @project)} - %li{class: ("active" if params[:state] == 'rejected')} - = link_to page_filter_path(state: 'rejected') do + %li{class: ("active" if params[:state] == 'closed')} + = link_to page_filter_path(state: 'closed') do = icon('ban') - #{state_filters_text_for(:rejected, @project)} + #{state_filters_text_for(:closed, @project)} - else %li{class: ("active" if params[:state] == 'closed')} = link_to page_filter_path(state: 'closed') do diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 1694818aef6..15d53499e03 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -9,7 +9,8 @@ .row %section.col-md-8 .header-with-avatar - = image_tag avatar_icon(@user.email, 90), class: "avatar avatar-tile s90", alt: '' + = link_to avatar_icon(@user.email), target: '_blank' do + = image_tag avatar_icon(@user.email, 90), class: "avatar avatar-tile s90", alt: '' %h3 = @user.name - if @user == current_user diff --git a/doc/install/installation.md b/doc/install/installation.md index ff0361c5e52..8b918cba133 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -368,6 +368,9 @@ Make sure to edit the config file to match your setup: # Change YOUR_SERVER_FQDN to the fully-qualified # domain name of your host serving GitLab. + # If using Ubuntu default nginx install: + # either remove the default_server from the listen line + # or else rm -f /etc/sites-enabled/default sudo editor /etc/nginx/sites-available/gitlab **Note:** If you want to use HTTPS, replace the `gitlab` Nginx config with `gitlab-ssl`. See [Using HTTPS](#using-https) for HTTPS configuration details. diff --git a/features/project/commits/comments.feature b/features/project/commits/comments.feature index c41075d7ad4..320f008abb6 100644 --- a/features/project/commits/comments.feature +++ b/features/project/commits/comments.feature @@ -39,6 +39,7 @@ Feature: Project Commits Comments @javascript Scenario: I can delete a comment Given I leave a comment like "XML attached" + Then I should see a comment saying "XML attached" And I delete a comment Then I should not see a comment saying "XML attached" diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature index eb091c291e9..d043badbc46 100644 --- a/features/project/merge_requests.feature +++ b/features/project/merge_requests.feature @@ -11,7 +11,7 @@ Feature: Project Merge Requests And I should not see "Feature NS-03" in merge requests Scenario: I should see rejected merge requests - Given I click link "Rejected" + Given I click link "Closed" Then I should see "Feature NS-03" in merge requests And I should not see "Bug NS-04" in merge requests diff --git a/features/project/wiki.feature b/features/project/wiki.feature index 7a70f348754..2ebfa3c1660 100644 --- a/features/project/wiki.feature +++ b/features/project/wiki.feature @@ -69,7 +69,7 @@ Feature: Project Wiki And I click on the "Pages" button Then I should see non-escaped link in the pages list - @javascript @focus + @javascript Scenario: Creating an invalid new page Given I create a New page with an invalid name Then I should see an error message diff --git a/features/steps/admin/broadcast_messages.rb b/features/steps/admin/broadcast_messages.rb index 2ecb6f0191a..f6daf852977 100644 --- a/features/steps/admin/broadcast_messages.rb +++ b/features/steps/admin/broadcast_messages.rb @@ -36,6 +36,6 @@ class Spinach::Features::AdminBroadcastMessages < Spinach::FeatureSteps step 'I should see a customized broadcast message' do expect(page).to have_content 'Application update from 4:00 CST to 5:00 CST' - expect(page).to have_selector %(div[style="background-color:#f2dede;color:#b94a48"]) + expect(page).to have_selector %(div[style="background-color: #f2dede; color: #b94a48"]) end end diff --git a/features/steps/admin/settings.rb b/features/steps/admin/settings.rb index 1c0b7a4b712..147a4bd7486 100644 --- a/features/steps/admin/settings.rb +++ b/features/steps/admin/settings.rb @@ -11,9 +11,9 @@ class Spinach::Features::AdminSettings < Spinach::FeatureSteps end step 'I should see application settings saved' do - expect(current_application_settings.gravatar_enabled).to be_false - expect(current_application_settings.home_page_url).to eq 'https://about.gitlab.com/' - expect(page).to have_content 'Application settings saved successfully' + expect(current_application_settings.gravatar_enabled).to be_falsey + expect(current_application_settings.home_page_url).to eq "https://about.gitlab.com/" + expect(page).to have_content "Application settings saved successfully" end step 'I click on "Service Templates"' do diff --git a/features/steps/dashboard/new_project.rb b/features/steps/dashboard/new_project.rb index b4ade65ee53..d4440c1fb4d 100644 --- a/features/steps/dashboard/new_project.rb +++ b/features/steps/dashboard/new_project.rb @@ -10,7 +10,7 @@ class Spinach::Features::NewProject < Spinach::FeatureSteps end step 'I see "New project" page' do - expect(page).to have_content("Project path") + expect(page).to have_content('Project path') end step 'I click on "Import project from GitHub"' do diff --git a/features/steps/groups.rb b/features/steps/groups.rb index c6c855a7c22..2812c5473e9 100644 --- a/features/steps/groups.rb +++ b/features/steps/groups.rb @@ -128,14 +128,14 @@ class Spinach::Features::Groups < Spinach::FeatureSteps end step 'I change group "Owned" avatar' do - attach_file(:group_avatar, File.join(Rails.root, 'public', 'gitlab_logo.png')) + attach_file(:group_avatar, File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif')) click_button "Save group" Group.find_by(name: "Owned").reload end step 'I should see new group "Owned" avatar' do expect(Group.find_by(name: "Owned").avatar).to be_instance_of AvatarUploader - expect(Group.find_by(name: "Owned").avatar.url).to eq "/uploads/group/avatar/#{ Group.find_by(name:"Owned").id }/gitlab_logo.png" + expect(Group.find_by(name: "Owned").avatar.url).to eq "/uploads/group/avatar/#{ Group.find_by(name:"Owned").id }/banana_sample.gif" end step 'I should see the "Remove avatar" button' do @@ -143,7 +143,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps end step 'I have group "Owned" avatar' do - attach_file(:group_avatar, File.join(Rails.root, 'public', 'gitlab_logo.png')) + attach_file(:group_avatar, File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif')) click_button "Save group" Group.find_by(name: "Owned").reload end @@ -154,7 +154,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps end step 'I should not see group "Owned" avatar' do - expect(Group.find_by(name: "Owned").avatar?).to be_false + expect(Group.find_by(name: "Owned").avatar?).to eq false end step 'I should not see the "Remove avatar" button' do diff --git a/features/steps/profile/profile.rb b/features/steps/profile/profile.rb index 10bd307320e..11e1163c352 100644 --- a/features/steps/profile/profile.rb +++ b/features/steps/profile/profile.rb @@ -27,14 +27,14 @@ class Spinach::Features::Profile < Spinach::FeatureSteps end step 'I change my avatar' do - attach_file(:user_avatar, File.join(Rails.root, 'public', 'gitlab_logo.png')) + attach_file(:user_avatar, File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif')) click_button "Save changes" @user.reload end step 'I should see new avatar' do expect(@user.avatar).to be_instance_of AvatarUploader - expect(@user.avatar.url).to eq "/uploads/user/avatar/#{ @user.id }/gitlab_logo.png" + expect(@user.avatar.url).to eq "/uploads/user/avatar/#{ @user.id }/banana_sample.gif" end step 'I should see the "Remove avatar" button' do @@ -42,7 +42,7 @@ class Spinach::Features::Profile < Spinach::FeatureSteps end step 'I have an avatar' do - attach_file(:user_avatar, File.join(Rails.root, 'public', 'gitlab_logo.png')) + attach_file(:user_avatar, File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif')) click_button "Save changes" @user.reload end @@ -53,7 +53,7 @@ class Spinach::Features::Profile < Spinach::FeatureSteps end step 'I should see my gravatar' do - expect(@user.avatar?).to be_false + expect(@user.avatar?).to eq false end step 'I should not see the "Remove avatar" button' do @@ -87,11 +87,15 @@ class Spinach::Features::Profile < Spinach::FeatureSteps end step "I should see a missing password error message" do - expect(page).to have_content "You must provide a valid current password" + page.within ".flash-container" do + expect(page).to have_content "You must provide a valid current password" + end end step "I should see a password error message" do - expect(page).to have_content "Password confirmation doesn't match" + page.within '.alert' do + expect(page).to have_content "Password confirmation doesn't match" + end end step 'I reset my token' do @@ -120,7 +124,7 @@ class Spinach::Features::Profile < Spinach::FeatureSteps step "I am not an ldap user" do current_user.identities.delete - expect(current_user.ldap_user?).to be_false + expect(current_user.ldap_user?).to eq false end step 'I redirected to expired password page' do diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index 62c64e60f6d..5684d661527 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -19,8 +19,8 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps click_link "All" end - step 'I click link "Rejected"' do - click_link "Rejected" + step 'I click link "Closed"' do + click_link "Closed" end step 'I should see merge request "Wiki Feature"' do @@ -31,8 +31,8 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps step 'I should see closed merge request "Bug NS-04"' do merge_request = MergeRequest.find_by!(title: "Bug NS-04") - expect(merge_request.closed?).to be_true - expect(page).to have_content "Rejected by" + expect(merge_request).to be_closed + expect(page).to have_content "Closed by" end step 'I should see merge request "Bug NS-04"' do @@ -125,7 +125,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps expect(buttons.count).to eq(2) buttons.each do |b| - expect(expect(b['href'])).not_to have_content('json') + expect(b['href']).not_to have_content('json') end end @@ -164,20 +164,26 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps end step 'I should see a discussion has started on diff' do - expect(page).to have_content "#{current_user.name} started a discussion" - expect(page).to have_content sample_commit.line_code_path - expect(page).to have_content "Line is wrong" + page.within(".notes .discussion") do + page.should have_content "#{current_user.name} started a discussion" + page.should have_content sample_commit.line_code_path + page.should have_content "Line is wrong" + end end step 'I should see a discussion has started on commit diff' do - expect(page).to have_content "#{current_user.name} started a discussion on commit" - expect(page).to have_content sample_commit.line_code_path - expect(page).to have_content "Line is wrong" + page.within(".notes .discussion") do + page.should have_content "#{current_user.name} started a discussion on commit" + page.should have_content sample_commit.line_code_path + page.should have_content "Line is wrong" + end end step 'I should see a discussion has started on commit' do - expect(page).to have_content "#{current_user.name} started a discussion on commit" - expect(page).to have_content "One comment to rule them all" + page.within(".notes .discussion") do + page.should have_content "#{current_user.name} started a discussion on commit" + page.should have_content "One comment to rule them all" + end end step 'merge request is mergeable' do @@ -206,7 +212,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps step 'I should see merged request' do page.within '.issue-box' do - expect(page).to have_content "Accepted" + expect(page).to have_content "Merged" end end @@ -329,12 +335,13 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps end def leave_comment(message) - page.within(".js-discussion-note-form") do + page.within(".js-discussion-note-form", visible: true) do fill_in "note_note", with: message click_button "Add Comment" end - - expect(page).to have_content message + page.within(".notes_holder", visible: true) do + expect(page).to have_content message + end end def init_diff_note_first_file diff --git a/features/steps/project/project.rb b/features/steps/project/project.rb index ee4c7cd0f06..b4a0ba1e27f 100644 --- a/features/steps/project/project.rb +++ b/features/steps/project/project.rb @@ -28,7 +28,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps step 'I change the project avatar' do attach_file( :project_avatar, - File.join(Rails.root, 'public', 'gitlab_logo.png') + File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif') ) click_button 'Save changes' @project.reload @@ -37,7 +37,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps step 'I should see new project avatar' do expect(@project.avatar).to be_instance_of AvatarUploader url = @project.avatar.url - expect(url).to eq "/uploads/project/avatar/#{ @project.id }/gitlab_logo.png" + expect(url).to eq "/uploads/project/avatar/#{ @project.id }/banana_sample.gif" end step 'I should see the "Remove avatar" button' do @@ -47,7 +47,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps step 'I have an project avatar' do attach_file( :project_avatar, - File.join(Rails.root, 'public', 'gitlab_logo.png') + File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif') ) click_button 'Save changes' @project.reload @@ -59,7 +59,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps end step 'I should see the default project avatar' do - expect(@project.avatar?).to be_false + expect(@project.avatar?).to eq false end step 'I should not see the "Remove avatar" button' do diff --git a/features/steps/search.rb b/features/steps/search.rb index fec5d9f0e4e..87893aa0205 100644 --- a/features/steps/search.rb +++ b/features/steps/search.rb @@ -52,7 +52,9 @@ class Spinach::Features::Search < Spinach::FeatureSteps end step 'I should see code results for project "Shop"' do - expect(page).to have_content 'Update capybara, rspec-rails, poltergeist to recent versions' + page.within('.results') do + page.should have_content 'Update capybara, rspec-rails, poltergeist to recent versions' + end end step 'I search for "Contibuting"' do @@ -71,7 +73,9 @@ class Spinach::Features::Search < Spinach::FeatureSteps end step 'I should see "Foo" link in the search results' do - expect(find(:css, '.search-results')).to have_link 'Foo' + page.within('.results') do + find(:css, '.search-results').should have_link 'Foo' + end end step 'I should not see "Bar" link in the search results' do @@ -79,7 +83,9 @@ class Spinach::Features::Search < Spinach::FeatureSteps end step 'I should see "test_wiki" link in the search results' do - expect(find(:css, '.search-results')).to have_link 'test_wiki.md' + page.within('.results') do + find(:css, '.search-results').should have_link 'test_wiki.md' + end end step 'project has Wiki content' do diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb index c4f89ca31c9..27a95aeb19a 100644 --- a/features/steps/shared/diff_note.rb +++ b/features/steps/shared/diff_note.rb @@ -20,11 +20,14 @@ module SharedDiffNote end step 'I leave a diff comment like "Typo, please fix"' do - click_diff_line(sample_commit.line_code) - page.within("#{diff_file_selector} form[rel$='#{sample_commit.line_code}']") do - fill_in "note[note]", with: "Typo, please fix" - find(".js-comment-button").trigger("click") - sleep 0.05 + page.within(diff_file_selector) do + click_diff_line(sample_commit.line_code) + + page.within("form[rel$='#{sample_commit.line_code}']") do + fill_in "note[note]", with: "Typo, please fix" + find(".js-comment-button").trigger("click") + sleep 0.05 + end end end @@ -45,28 +48,37 @@ module SharedDiffNote end step 'I preview a diff comment text like "Should fix it :smile:"' do - click_diff_line(sample_commit.line_code) - page.within("#{diff_file_selector} form[rel$='#{sample_commit.line_code}']") do - fill_in "note[note]", with: "Should fix it :smile:" - find('.js-md-preview-button').click + page.within(diff_file_selector) do + click_diff_line(sample_commit.line_code) + + page.within("form[rel$='#{sample_commit.line_code}']") do + fill_in "note[note]", with: "Should fix it :smile:" + find('.js-md-preview-button').click + end end end step 'I preview another diff comment text like "DRY this up"' do - click_diff_line(sample_commit.del_line_code) + page.within(diff_file_selector) do + click_diff_line(sample_commit.del_line_code) - page.within("#{diff_file_selector} form[rel$='#{sample_commit.del_line_code}']") do - fill_in "note[note]", with: "DRY this up" - find('.js-md-preview-button').click + page.within("form[rel$='#{sample_commit.del_line_code}']") do + fill_in "note[note]", with: "DRY this up" + find('.js-md-preview-button').click + end end end step 'I open a diff comment form' do - click_diff_line(sample_commit.line_code) + page.within(diff_file_selector) do + click_diff_line(sample_commit.line_code) + end end step 'I open another diff comment form' do - click_diff_line(sample_commit.del_line_code) + page.within(diff_file_selector) do + click_diff_line(sample_commit.del_line_code) + end end step 'I write a diff comment like ":-1: I don\'t like this"' do @@ -194,7 +206,7 @@ module SharedDiffNote end def diff_file_selector - ".diff-file:nth-of-type(1)" + '.diff-file:nth-of-type(1)' end def click_diff_line(code) diff --git a/features/steps/shared/note.rb b/features/steps/shared/note.rb index b2675546a14..f6aabfefeff 100644 --- a/features/steps/shared/note.rb +++ b/features/steps/shared/note.rb @@ -2,8 +2,10 @@ module SharedNote include Spinach::DSL step 'I delete a comment' do - find('.note').hover - find(".js-note-delete").click + page.within('.notes') do + find('.note').hover + find(".js-note-delete").click + end end step 'I haven\'t written any comment text' do @@ -16,7 +18,6 @@ module SharedNote page.within(".js-main-target-form") do fill_in "note[note]", with: "XML attached" click_button "Add Comment" - sleep 0.05 end end @@ -123,13 +124,14 @@ module SharedNote end step 'I edit the last comment with a +1' do - find(".note").hover - find('.js-note-edit').click + page.within(".notes") do + find(".note").hover + find('.js-note-edit').click + end page.within(".current-note-edit-form") do fill_in 'note[note]', with: '+1 Awesome!' click_button 'Save Comment' - sleep 0.05 end end diff --git a/features/support/env.rb b/features/support/env.rb index d4a878ea4ce..672251af084 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -25,6 +25,7 @@ WebMock.allow_net_connect! Spinach.hooks.before_run do include RSpec::Mocks::ExampleMethods + RSpec::Mocks.setup TestEnv.init(mailer: false) include FactoryGirl::Syntax::Methods diff --git a/lib/support/nginx/gitlab b/lib/support/nginx/gitlab index 4688a527eba..edb987875df 100644 --- a/lib/support/nginx/gitlab +++ b/lib/support/nginx/gitlab @@ -40,6 +40,10 @@ upstream gitlab { ## Normal HTTP host server { + ## Either remove "default_server" from the listen line below, + ## or delete the /etc/nginx/sites-enabled/default file. This will cause gitlab + ## to be served if you visit any address that your server responds to, eg. + ## the ip address of the server (http://x.x.x.x/)n 0.0.0.0:80 default_server; listen 0.0.0.0:80 default_server; listen [::]:80 default_server; server_name YOUR_SERVER_FQDN; ## Replace this with something like gitlab.example.com diff --git a/lib/support/nginx/gitlab-ssl b/lib/support/nginx/gitlab-ssl index 5c94ec63432..766559b49f6 100644 --- a/lib/support/nginx/gitlab-ssl +++ b/lib/support/nginx/gitlab-ssl @@ -44,6 +44,10 @@ upstream gitlab { ## Redirects all HTTP traffic to the HTTPS host server { + ## Either remove "default_server" from the listen line below, + ## or delete the /etc/nginx/sites-enabled/default file. This will cause gitlab + ## to be served if you visit any address that your server responds to, eg. + ## the ip address of the server (http://x.x.x.x/) listen 0.0.0.0:80; listen [::]:80 ipv6only=on default_server; server_name YOUR_SERVER_FQDN; ## Replace this with something like gitlab.example.com diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 1ea1227b28b..9ad9cb41cc1 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -16,7 +16,7 @@ describe AutocompleteController do let(:body) { JSON.parse(response.body) } it { expect(body).to be_kind_of(Array) } - it { expect(body.size).to eq(1) } + it { expect(body.size).to eq 1 } it { expect(body.first["username"]).to eq user.username } end @@ -33,7 +33,7 @@ describe AutocompleteController do let(:body) { JSON.parse(response.body) } it { expect(body).to be_kind_of(Array) } - it { expect(body.size).to eq(1) } + it { expect(body.size).to eq 1 } it { expect(body.first["username"]).to eq user.username } end @@ -46,6 +46,6 @@ describe AutocompleteController do let(:body) { JSON.parse(response.body) } it { expect(body).to be_kind_of(Array) } - it { expect(body.size).to eq(User.count) } + it { expect(body.size).to eq User.count } end end diff --git a/spec/controllers/commit_controller_spec.rb b/spec/controllers/commit_controller_spec.rb index 2dfc8e2821f..bb3d87f3840 100644 --- a/spec/controllers/commit_controller_spec.rb +++ b/spec/controllers/commit_controller_spec.rb @@ -49,10 +49,10 @@ describe Projects::CommitController do project_id: project.to_param, id: commit.id, format: format) - expect(response.body).to_not include('&') - expect(response.body).to_not include('>') - expect(response.body).to_not include('<') - expect(response.body).to_not include('"') + expect(response.body).not_to include('&') + expect(response.body).not_to include('>') + expect(response.body).not_to include('<') + expect(response.body).not_to include('"') end end diff --git a/spec/controllers/import/bitbucket_controller_spec.rb b/spec/controllers/import/bitbucket_controller_spec.rb index ddeeaf74fcb..d5d9310e603 100644 --- a/spec/controllers/import/bitbucket_controller_spec.rb +++ b/spec/controllers/import/bitbucket_controller_spec.rb @@ -1,11 +1,14 @@ require 'spec_helper' +require_relative 'import_spec_helper' describe Import::BitbucketController do + include ImportSpecHelper + let(:user) { create(:user, bitbucket_access_token: 'asd123', bitbucket_access_token_secret: "sekret") } before do sign_in(user) - controller.stub(:bitbucket_import_enabled?).and_return(true) + allow(controller).to receive(:bitbucket_import_enabled?).and_return(true) end describe "GET callback" do @@ -17,8 +20,9 @@ describe Import::BitbucketController do token = "asdasd12345" secret = "sekrettt" access_token = double(token: token, secret: secret) - Gitlab::BitbucketImport::Client.any_instance.stub(:get_token).and_return(access_token) - Gitlab.config.omniauth.providers << OpenStruct.new(app_id: "asd123", app_secret: "asd123", name: "bitbucket") + allow_any_instance_of(Gitlab::BitbucketImport::Client). + to receive(:get_token).and_return(access_token) + stub_omniauth_provider('bitbucket') get :callback @@ -35,7 +39,7 @@ describe Import::BitbucketController do it "assigns variables" do @project = create(:project, import_type: 'bitbucket', creator_id: user.id) - controller.stub_chain(:client, :projects).and_return([@repo]) + stub_client(projects: [@repo]) get :status @@ -45,7 +49,7 @@ describe Import::BitbucketController do it "does not show already added project" do @project = create(:project, import_type: 'bitbucket', creator_id: user.id, import_source: 'asd/vim') - controller.stub_chain(:client, :projects).and_return([@repo]) + stub_client(projects: [@repo]) get :status @@ -70,8 +74,7 @@ describe Import::BitbucketController do to receive(:new).with(bitbucket_repo, user). and_return(double(execute: true)) - controller.stub_chain(:client, :user).and_return(bitbucket_user) - controller.stub_chain(:client, :project).and_return(bitbucket_repo) + stub_client(user: bitbucket_user, project: bitbucket_repo) end context "when the repository owner is the Bitbucket user" do diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index 5f2a09280b2..0bc14059a35 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -1,11 +1,14 @@ require 'spec_helper' +require_relative 'import_spec_helper' describe Import::GithubController do + include ImportSpecHelper + let(:user) { create(:user, github_access_token: 'asd123') } before do sign_in(user) - controller.stub(:github_import_enabled?).and_return(true) + allow(controller).to receive(:github_import_enabled?).and_return(true) end describe "GET callback" do @@ -13,9 +16,7 @@ describe Import::GithubController do token = "asdasd12345" allow_any_instance_of(Gitlab::GithubImport::Client). to receive(:get_token).and_return(token) - Gitlab.config.omniauth.providers << OpenStruct.new(app_id: 'asd123', - app_secret: 'asd123', - name: 'github') + stub_omniauth_provider('github') get :callback @@ -33,9 +34,7 @@ describe Import::GithubController do it "assigns variables" do @project = create(:project, import_type: 'github', creator_id: user.id) - controller.stub_chain(:client, :repos).and_return([@repo]) - controller.stub_chain(:client, :orgs).and_return([@org]) - controller.stub_chain(:client, :org_repos).with(@org.login).and_return([@org_repo]) + stub_client(repos: [@repo], orgs: [@org], org_repos: [@org_repo]) get :status @@ -45,8 +44,7 @@ describe Import::GithubController do it "does not show already added project" do @project = create(:project, import_type: 'github', creator_id: user.id, import_source: 'asd/vim') - controller.stub_chain(:client, :repos).and_return([@repo]) - controller.stub_chain(:client, :orgs).and_return([]) + stub_client(repos: [@repo], orgs: []) get :status @@ -67,8 +65,7 @@ describe Import::GithubController do end before do - controller.stub_chain(:client, :user).and_return(github_user) - controller.stub_chain(:client, :repo).and_return(github_repo) + stub_client(user: github_user, repo: github_repo) end context "when the repository owner is the GitHub user" do diff --git a/spec/controllers/import/gitlab_controller_spec.rb b/spec/controllers/import/gitlab_controller_spec.rb index a0c5ce1fe5b..4bc67c86703 100644 --- a/spec/controllers/import/gitlab_controller_spec.rb +++ b/spec/controllers/import/gitlab_controller_spec.rb @@ -1,18 +1,22 @@ require 'spec_helper' +require_relative 'import_spec_helper' describe Import::GitlabController do + include ImportSpecHelper + let(:user) { create(:user, gitlab_access_token: 'asd123') } before do sign_in(user) - controller.stub(:gitlab_import_enabled?).and_return(true) + allow(controller).to receive(:gitlab_import_enabled?).and_return(true) end describe "GET callback" do it "updates access token" do token = "asdasd12345" - Gitlab::GitlabImport::Client.any_instance.stub_chain(:client, :auth_code, :get_token, :token).and_return(token) - Gitlab.config.omniauth.providers << OpenStruct.new(app_id: "asd123", app_secret: "asd123", name: "gitlab") + allow_any_instance_of(Gitlab::GitlabImport::Client). + to receive(:get_token).and_return(token) + stub_omniauth_provider('gitlab') get :callback @@ -28,7 +32,7 @@ describe Import::GitlabController do it "assigns variables" do @project = create(:project, import_type: 'gitlab', creator_id: user.id) - controller.stub_chain(:client, :projects).and_return([@repo]) + stub_client(projects: [@repo]) get :status @@ -38,7 +42,7 @@ describe Import::GitlabController do it "does not show already added project" do @project = create(:project, import_type: 'gitlab', creator_id: user.id, import_source: 'asd/vim') - controller.stub_chain(:client, :projects).and_return([@repo]) + stub_client(projects: [@repo]) get :status @@ -62,8 +66,7 @@ describe Import::GitlabController do end before do - controller.stub_chain(:client, :user).and_return(gitlab_user) - controller.stub_chain(:client, :project).and_return(gitlab_repo) + stub_client(user: gitlab_user, project: gitlab_repo) end context "when the repository owner is the GitLab.com user" do diff --git a/spec/controllers/import/gitorious_controller_spec.rb b/spec/controllers/import/gitorious_controller_spec.rb index 07c9484bf1a..7cb1b85a46d 100644 --- a/spec/controllers/import/gitorious_controller_spec.rb +++ b/spec/controllers/import/gitorious_controller_spec.rb @@ -1,6 +1,9 @@ require 'spec_helper' +require_relative 'import_spec_helper' describe Import::GitoriousController do + include ImportSpecHelper + let(:user) { create(:user) } before do @@ -30,7 +33,7 @@ describe Import::GitoriousController do it "assigns variables" do @project = create(:project, import_type: 'gitorious', creator_id: user.id) - controller.stub_chain(:client, :repos).and_return([@repo]) + stub_client(repos: [@repo]) get :status @@ -40,7 +43,7 @@ describe Import::GitoriousController do it "does not show already added project" do @project = create(:project, import_type: 'gitorious', creator_id: user.id, import_source: 'asd/vim') - controller.stub_chain(:client, :repos).and_return([@repo]) + stub_client(repos: [@repo]) get :status @@ -59,7 +62,7 @@ describe Import::GitoriousController do expect(Gitlab::GitoriousImport::ProjectCreator). to receive(:new).with(@repo, namespace, user). and_return(double(execute: true)) - controller.stub_chain(:client, :repo).and_return(@repo) + stub_client(repo: @repo) post :create, format: :js end diff --git a/spec/controllers/import/google_code_controller_spec.rb b/spec/controllers/import/google_code_controller_spec.rb index 78c0f5079cc..66088139a69 100644 --- a/spec/controllers/import/google_code_controller_spec.rb +++ b/spec/controllers/import/google_code_controller_spec.rb @@ -1,6 +1,9 @@ require 'spec_helper' +require_relative 'import_spec_helper' describe Import::GoogleCodeController do + include ImportSpecHelper + let(:user) { create(:user) } let(:dump_file) { fixture_file_upload(Rails.root + 'spec/fixtures/GoogleCodeProjectHosting.json', 'application/json') } @@ -21,13 +24,12 @@ describe Import::GoogleCodeController do describe "GET status" do before do @repo = OpenStruct.new(name: 'vim') - controller.stub_chain(:client, :valid?).and_return(true) + stub_client(valid?: true) end it "assigns variables" do @project = create(:project, import_type: 'google_code', creator_id: user.id) - controller.stub_chain(:client, :repos).and_return([@repo]) - controller.stub_chain(:client, :incompatible_repos).and_return([]) + stub_client(repos: [@repo], incompatible_repos: []) get :status @@ -38,8 +40,7 @@ describe Import::GoogleCodeController do it "does not show already added project" do @project = create(:project, import_type: 'google_code', creator_id: user.id, import_source: 'vim') - controller.stub_chain(:client, :repos).and_return([@repo]) - controller.stub_chain(:client, :incompatible_repos).and_return([]) + stub_client(repos: [@repo], incompatible_repos: []) get :status @@ -48,8 +49,7 @@ describe Import::GoogleCodeController do end it "does not show any invalid projects" do - controller.stub_chain(:client, :repos).and_return([]) - controller.stub_chain(:client, :incompatible_repos).and_return([@repo]) + stub_client(repos: [], incompatible_repos: [@repo]) get :status diff --git a/spec/controllers/import/import_spec_helper.rb b/spec/controllers/import/import_spec_helper.rb new file mode 100644 index 00000000000..9d7648e25a7 --- /dev/null +++ b/spec/controllers/import/import_spec_helper.rb @@ -0,0 +1,33 @@ +require 'ostruct' + +# Helper methods for controller specs in the Import namespace +# +# Must be included manually. +module ImportSpecHelper + # Stub `controller` to return a null object double with the provided messages + # when `client` is called + # + # Examples: + # + # stub_client(foo: %w(foo)) + # + # controller.client.foo # => ["foo"] + # controller.client.bar.baz.foo # => ["foo"] + # + # Returns the client double + def stub_client(messages = {}) + client = double('client', messages).as_null_object + allow(controller).to receive(:client).and_return(client) + + client + end + + def stub_omniauth_provider(name) + provider = OpenStruct.new( + name: name, + app_id: 'asd123', + app_secret: 'asd123' + ) + Gitlab.config.omniauth.providers << provider + end +end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index bda931666f7..b8db8591709 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -52,10 +52,10 @@ describe Projects::MergeRequestsController do id: merge_request.iid, format: format) - expect(response.body).to_not include('&') - expect(response.body).to_not include('>') - expect(response.body).to_not include('<') - expect(response.body).to_not include('"') + expect(response.body).not_to include('&') + expect(response.body).not_to include('>') + expect(response.body).not_to include('<') + expect(response.body).not_to include('"') end end diff --git a/spec/features/gitlab_flavored_markdown_spec.rb b/spec/features/gitlab_flavored_markdown_spec.rb index 16d1ca55f8d..0c1bc53cdb5 100644 --- a/spec/features/gitlab_flavored_markdown_spec.rb +++ b/spec/features/gitlab_flavored_markdown_spec.rb @@ -11,7 +11,8 @@ describe "GitLab Flavored Markdown", feature: true do end before do - Commit.any_instance.stub(title: "fix #{issue.to_reference}\n\nask #{fred.to_reference} for details") + allow_any_instance_of(Commit).to receive(:title). + and_return("fix #{issue.to_reference}\n\nask #{fred.to_reference} for details") end let(:commit) { project.commit } diff --git a/spec/features/profile_spec.rb b/spec/features/profile_spec.rb index 3d36a3c02d0..9fe2e610555 100644 --- a/spec/features/profile_spec.rb +++ b/spec/features/profile_spec.rb @@ -9,7 +9,8 @@ describe 'Profile account page', feature: true do describe 'when signup is enabled' do before do - ApplicationSetting.any_instance.stub(signup_enabled?: true) + allow_any_instance_of(ApplicationSetting). + to receive(:signup_enabled?).and_return(true) visit profile_account_path end @@ -23,7 +24,8 @@ describe 'Profile account page', feature: true do describe 'when signup is disabled' do before do - ApplicationSetting.any_instance.stub(signup_enabled?: false) + allow_any_instance_of(ApplicationSetting). + to receive(:signup_enabled?).and_return(false) visit profile_account_path end diff --git a/spec/features/profiles/preferences_spec.rb b/spec/features/profiles/preferences_spec.rb index 69d15f41706..03e78c533db 100644 --- a/spec/features/profiles/preferences_spec.rb +++ b/spec/features/profiles/preferences_spec.rb @@ -75,7 +75,7 @@ describe 'Profile > Preferences' do end def expect_preferences_saved_message - within('.flash-container') do + page.within('.flash-container') do expect(page).to have_content('Preferences saved.') end end diff --git a/spec/features/security/profile_access_spec.rb b/spec/features/security/profile_access_spec.rb index 2b09771851e..8f7a9606262 100644 --- a/spec/features/security/profile_access_spec.rb +++ b/spec/features/security/profile_access_spec.rb @@ -6,7 +6,7 @@ describe "Profile access", feature: true do end describe "GET /login" do - it { expect(new_user_session_path).not_to be_404_for :visitor } + it { expect(new_user_session_path).not_to be_not_found_for :visitor } end describe "GET /profile/keys" do diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 1297e789793..8fd3d8f407b 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -40,15 +40,15 @@ describe ApplicationHelper do end describe 'project_icon' do - avatar_file_path = File.join(Rails.root, 'public', 'gitlab_logo.png') + avatar_file_path = File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif') it 'should return an url for the avatar' do project = create(:project) project.avatar = File.open(avatar_file_path) project.save! - avatar_url = "http://localhost/uploads/project/avatar/#{ project.id }/gitlab_logo.png" + avatar_url = "http://localhost/uploads/project/avatar/#{ project.id }/banana_sample.gif" expect(project_icon("#{project.namespace.to_param}/#{project.to_param}").to_s).to eq( - "<img alt=\"Gitlab logo\" src=\"#{avatar_url}\" />" + "<img alt=\"Banana sample\" src=\"#{avatar_url}\" />" ) end @@ -65,25 +65,25 @@ describe ApplicationHelper do end describe 'avatar_icon' do - avatar_file_path = File.join(Rails.root, 'public', 'gitlab_logo.png') + avatar_file_path = File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif') it 'should return an url for the avatar' do user = create(:user) user.avatar = File.open(avatar_file_path) user.save! expect(avatar_icon(user.email).to_s). - to match("/uploads/user/avatar/#{ user.id }/gitlab_logo.png") + to match("/uploads/user/avatar/#{ user.id }/banana_sample.gif") end it 'should return an url for the avatar with relative url' do - Gitlab.config.gitlab.stub(relative_url_root: '/gitlab') - Gitlab.config.gitlab.stub(url: Settings.send(:build_gitlab_url)) + allow(Gitlab.config.gitlab).to receive(:relative_url_root).and_return('/gitlab') + allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url)) user = create(:user) user.avatar = File.open(avatar_file_path) user.save! expect(avatar_icon(user.email).to_s). - to match("/gitlab/uploads/user/avatar/#{ user.id }/gitlab_logo.png") + to match("/gitlab/uploads/user/avatar/#{ user.id }/banana_sample.gif") end it 'should call gravatar_icon when no avatar is present' do @@ -97,7 +97,7 @@ describe ApplicationHelper do let(:user_email) { 'user@email.com' } it 'should return a generic avatar path when Gravatar is disabled' do - ApplicationSetting.any_instance.stub(gravatar_enabled?: false) + allow_any_instance_of(ApplicationSetting).to receive(:gravatar_enabled?).and_return(false) expect(gravatar_icon(user_email)).to match('no_avatar.png') end @@ -106,13 +106,13 @@ describe ApplicationHelper do end it 'should return default gravatar url' do - Gitlab.config.gitlab.stub(https: false) + allow(Gitlab.config.gitlab).to receive(:https).and_return(false) url = 'http://www.gravatar.com/avatar/b58c6f14d292556214bd64909bcdb118' expect(gravatar_icon(user_email)).to match(url) end it 'should use SSL when appropriate' do - Gitlab.config.gitlab.stub(https: true) + allow(Gitlab.config.gitlab).to receive(:https).and_return(true) expect(gravatar_icon(user_email)).to match('https://secure.gravatar.com') end diff --git a/spec/helpers/broadcast_messages_helper_spec.rb b/spec/helpers/broadcast_messages_helper_spec.rb index f6df12662bb..c7c6f45d144 100644 --- a/spec/helpers/broadcast_messages_helper_spec.rb +++ b/spec/helpers/broadcast_messages_helper_spec.rb @@ -2,20 +2,20 @@ require 'spec_helper' describe BroadcastMessagesHelper do describe 'broadcast_styling' do - let(:broadcast_message) { double(color: "", font: "") } + let(:broadcast_message) { double(color: '', font: '') } context "default style" do it "should have no style" do - expect(broadcast_styling(broadcast_message)).to match('') + expect(broadcast_styling(broadcast_message)).to eq '' end end - context "customiezd style" do - before { broadcast_message.stub(color: "#f2dede", font: "#b94a48") } + context "customized style" do + let(:broadcast_message) { double(color: "#f2dede", font: '#b94a48') } it "should have a customized style" do expect(broadcast_styling(broadcast_message)). - to match('background-color:#f2dede;color:#b94a48') + to match('background-color: #f2dede; color: #b94a48') end end end diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index e0be2df0e5e..7c96a74e581 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -48,19 +48,19 @@ describe DiffHelper do end it 'should return only the first file if the diff line count in the 2nd file takes the total beyond safe limits' do - diffs[1].diff.stub(lines: [""] * 4999) #simulate 4999 lines + allow(diffs[1].diff).to receive(:lines).and_return([""] * 4999) #simulate 4999 lines expect(safe_diff_files(diffs).length).to eq(1) end it 'should return all files from a commit that is beyond safe limit for numbers of lines if force diff is true' do allow(controller).to receive(:params) { { force_show_diff: true } } - diffs[1].diff.stub(lines: [""] * 4999) #simulate 4999 lines + allow(diffs[1].diff).to receive(:lines).and_return([""] * 4999) #simulate 4999 lines expect(safe_diff_files(diffs).length).to eq(2) end it 'should return only the first file if the diff line count in the 2nd file takes the total beyond hard limits' do allow(controller).to receive(:params) { { force_show_diff: true } } - diffs[1].diff.stub(lines: [""] * 49999) #simulate 49999 lines + allow(diffs[1].diff).to receive(:lines).and_return([""] * 49999) #simulate 49999 lines expect(safe_diff_files(diffs).length).to eq(1) end diff --git a/spec/helpers/groups_helper.rb b/spec/helpers/groups_helper.rb index 3e99ab84ec9..5d174460681 100644 --- a/spec/helpers/groups_helper.rb +++ b/spec/helpers/groups_helper.rb @@ -2,14 +2,14 @@ require 'spec_helper' describe GroupsHelper do describe 'group_icon' do - avatar_file_path = File.join(Rails.root, 'public', 'gitlab_logo.png') + avatar_file_path = File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif') it 'should return an url for the avatar' do group = create(:group) group.avatar = File.open(avatar_file_path) group.save! expect(group_icon(group.path).to_s). - to match("/uploads/group/avatar/#{ group.id }/gitlab_logo.png") + to match("/uploads/group/avatar/#{ group.id }/banana_sample.gif") end it 'should give default avatar_icon when no avatar is present' do diff --git a/spec/helpers/notifications_helper_spec.rb b/spec/helpers/notifications_helper_spec.rb index 482cb33e94f..f1aba4cfdf3 100644 --- a/spec/helpers/notifications_helper_spec.rb +++ b/spec/helpers/notifications_helper_spec.rb @@ -1,14 +1,11 @@ require 'spec_helper' describe NotificationsHelper do - include FontAwesome::Rails::IconHelper - include IconsHelper - describe 'notification_icon' do let(:notification) { double(disabled?: false, participating?: false, watch?: false) } context "disabled notification" do - before { notification.stub(disabled?: true) } + before { allow(notification).to receive(:disabled?).and_return(true) } it "has a red icon" do expect(notification_icon(notification)).to match('class="fa fa-volume-off ns-mute"') @@ -16,7 +13,7 @@ describe NotificationsHelper do end context "participating notification" do - before { notification.stub(participating?: true) } + before { allow(notification).to receive(:participating?).and_return(true) } it "has a blue icon" do expect(notification_icon(notification)).to match('class="fa fa-volume-down ns-part"') @@ -24,7 +21,7 @@ describe NotificationsHelper do end context "watched notification" do - before { notification.stub(watch?: true) } + before { allow(notification).to receive(:watch?).and_return(true) } it "has a green icon" do expect(notification_icon(notification)).to match('class="fa fa-volume-up ns-watch"') diff --git a/spec/helpers/submodule_helper_spec.rb b/spec/helpers/submodule_helper_spec.rb index e98b75afabc..a7abf9d3839 100644 --- a/spec/helpers/submodule_helper_spec.rb +++ b/spec/helpers/submodule_helper_spec.rb @@ -14,41 +14,41 @@ describe SubmoduleHelper do context 'submodule on self' do before do - Gitlab.config.gitlab.stub(protocol: 'http') # set this just to be sure + allow(Gitlab.config.gitlab).to receive(:protocol).and_return('http') # set this just to be sure end it 'should detect ssh on standard port' do - Gitlab.config.gitlab_shell.stub(ssh_port: 22) # set this just to be sure - Gitlab.config.gitlab_shell.stub(ssh_path_prefix: Settings.send(:build_gitlab_shell_ssh_path_prefix)) + allow(Gitlab.config.gitlab_shell).to receive(:ssh_port).and_return(22) # set this just to be sure + allow(Gitlab.config.gitlab_shell).to receive(:ssh_path_prefix).and_return(Settings.send(:build_gitlab_shell_ssh_path_prefix)) stub_url([ config.user, '@', config.host, ':gitlab-org/gitlab-ce.git' ].join('')) expect(submodule_links(submodule_item)).to eq([ namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash') ]) end it 'should detect ssh on non-standard port' do - Gitlab.config.gitlab_shell.stub(ssh_port: 2222) - Gitlab.config.gitlab_shell.stub(ssh_path_prefix: Settings.send(:build_gitlab_shell_ssh_path_prefix)) + allow(Gitlab.config.gitlab_shell).to receive(:ssh_port).and_return(2222) + allow(Gitlab.config.gitlab_shell).to receive(:ssh_path_prefix).and_return(Settings.send(:build_gitlab_shell_ssh_path_prefix)) stub_url([ 'ssh://', config.user, '@', config.host, ':2222/gitlab-org/gitlab-ce.git' ].join('')) expect(submodule_links(submodule_item)).to eq([ namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash') ]) end it 'should detect http on standard port' do - Gitlab.config.gitlab.stub(port: 80) - Gitlab.config.gitlab.stub(url: Settings.send(:build_gitlab_url)) + allow(Gitlab.config.gitlab).to receive(:port).and_return(80) + allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url)) stub_url([ 'http://', config.host, '/gitlab-org/gitlab-ce.git' ].join('')) expect(submodule_links(submodule_item)).to eq([ namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash') ]) end it 'should detect http on non-standard port' do - Gitlab.config.gitlab.stub(port: 3000) - Gitlab.config.gitlab.stub(url: Settings.send(:build_gitlab_url)) + allow(Gitlab.config.gitlab).to receive(:port).and_return(3000) + allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url)) stub_url([ 'http://', config.host, ':3000/gitlab-org/gitlab-ce.git' ].join('')) expect(submodule_links(submodule_item)).to eq([ namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash') ]) end it 'should work with relative_url_root' do - Gitlab.config.gitlab.stub(port: 80) # set this just to be sure - Gitlab.config.gitlab.stub(relative_url_root: '/gitlab/root') - Gitlab.config.gitlab.stub(url: Settings.send(:build_gitlab_url)) + allow(Gitlab.config.gitlab).to receive(:port).and_return(80) # set this just to be sure + allow(Gitlab.config.gitlab).to receive(:relative_url_root).and_return('/gitlab/root') + allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url)) stub_url([ 'http://', config.host, '/gitlab/root/gitlab-org/gitlab-ce.git' ].join('')) expect(submodule_links(submodule_item)).to eq([ namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash') ]) end @@ -156,6 +156,6 @@ describe SubmoduleHelper do end def stub_url(url) - repo.stub(submodule_url_for: url) + allow(repo).to receive(:submodule_url_for).and_return(url) end end diff --git a/spec/lib/extracts_path_spec.rb b/spec/lib/extracts_path_spec.rb index 85f35d9da66..f077c80d478 100644 --- a/spec/lib/extracts_path_spec.rb +++ b/spec/lib/extracts_path_spec.rb @@ -9,8 +9,11 @@ describe ExtractsPath do before do @project = project - project.stub(repository: double(ref_names: ['master', 'foo/bar/baz', 'v1.0.0', 'v2.0.0'])) - project.stub(path_with_namespace: 'gitlab/gitlab-ci') + + repo = double(ref_names: ['master', 'foo/bar/baz', 'v1.0.0', 'v2.0.0']) + allow(project).to receive(:repository).and_return(repo) + allow(project).to receive(:path_with_namespace). + and_return('gitlab/gitlab-ci') end describe '#assign_ref' do diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 95fc7e16a11..72806bebe1f 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -27,16 +27,18 @@ describe Gitlab::Auth do it "should not find user with invalid password" do password = 'wrong' - expect( gl_auth.find(username, password) ).to_not eql user + expect( gl_auth.find(username, password) ).not_to eql user end it "should not find user with invalid login" do user = 'wrong' - expect( gl_auth.find(username, password) ).to_not eql user + expect( gl_auth.find(username, password) ).not_to eql user end context "with ldap enabled" do - before { Gitlab::LDAP::Config.stub(enabled?: true) } + before do + allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true) + end it "tries to autheticate with db before ldap" do expect(Gitlab::LDAP::Authentication).not_to receive(:login) diff --git a/spec/lib/gitlab/backend/shell_spec.rb b/spec/lib/gitlab/backend/shell_spec.rb index 27279465c1a..b6d04330599 100644 --- a/spec/lib/gitlab/backend/shell_spec.rb +++ b/spec/lib/gitlab/backend/shell_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::Shell do let(:gitlab_shell) { Gitlab::Shell.new } before do - Project.stub(find: project) + allow(Project).to receive(:find).and_return(project) end it { is_expected.to respond_to :add_key } diff --git a/spec/lib/gitlab/google_code_import/client_spec.rb b/spec/lib/gitlab/google_code_import/client_spec.rb index a66b811e0fd..6aa4428f367 100644 --- a/spec/lib/gitlab/google_code_import/client_spec.rb +++ b/spec/lib/gitlab/google_code_import/client_spec.rb @@ -15,7 +15,7 @@ describe Gitlab::GoogleCodeImport::Client do let(:raw_data) { "No clue" } it "returns true" do - expect(subject).to_not be_valid + expect(subject).not_to be_valid end end end diff --git a/spec/lib/gitlab/google_code_import/importer_spec.rb b/spec/lib/gitlab/google_code_import/importer_spec.rb index 3879a6ab898..c53ddeb87b5 100644 --- a/spec/lib/gitlab/google_code_import/importer_spec.rb +++ b/spec/lib/gitlab/google_code_import/importer_spec.rb @@ -24,7 +24,7 @@ describe Gitlab::GoogleCodeImport::Importer do subject.execute %w(New NeedInfo Accepted Wishlist Started Fixed Invalid Duplicate WontFix Incomplete).each do |status| - expect(project.labels.find_by(name: "Status: #{status}")).to_not be_nil + expect(project.labels.find_by(name: "Status: #{status}")).not_to be_nil end end @@ -38,7 +38,7 @@ describe Gitlab::GoogleCodeImport::Importer do Component-Systray Component-Clock Component-Launcher Component-Tint2conf Component-Docs Component-New ).each do |label| label.sub!("-", ": ") - expect(project.labels.find_by(name: label)).to_not be_nil + expect(project.labels.find_by(name: label)).not_to be_nil end end @@ -46,7 +46,7 @@ describe Gitlab::GoogleCodeImport::Importer do subject.execute issue = project.issues.first - expect(issue).to_not be_nil + expect(issue).not_to be_nil expect(issue.iid).to eq(169) expect(issue.author).to eq(project.creator) expect(issue.assignee).to eq(mapped_user) @@ -71,7 +71,7 @@ describe Gitlab::GoogleCodeImport::Importer do subject.execute note = project.issues.first.notes.first - expect(note).to_not be_nil + expect(note).not_to be_nil expect(note.note).to include("Comment 1") expect(note.note).to include("@#{mapped_user.username}") expect(note.note).to include("November 18, 2009 05:14") diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index 038ac7e0d75..c38f212b405 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -8,16 +8,24 @@ describe Gitlab::LDAP::Access do subject { access.allowed? } context 'when the user cannot be found' do - before { Gitlab::LDAP::Person.stub(find_by_dn: nil) } + before do + allow(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(nil) + end it { is_expected.to be_falsey } end context 'when the user is found' do - before { Gitlab::LDAP::Person.stub(find_by_dn: :ldap_user) } + before do + allow(Gitlab::LDAP::Person). + to receive(:find_by_dn).and_return(:ldap_user) + end context 'and the user is disabled via active directory' do - before { Gitlab::LDAP::Person.stub(disabled_via_active_directory?: true) } + before do + allow(Gitlab::LDAP::Person). + to receive(:disabled_via_active_directory?).and_return(true) + end it { is_expected.to be_falsey } @@ -30,8 +38,9 @@ describe Gitlab::LDAP::Access do context 'and has no disabled flag in active diretory' do before do user.block - - Gitlab::LDAP::Person.stub(disabled_via_active_directory?: false) + + allow(Gitlab::LDAP::Person). + to receive(:disabled_via_active_directory?).and_return(false) end it { is_expected.to be_truthy } @@ -39,7 +48,8 @@ describe Gitlab::LDAP::Access do context 'when auto-created users are blocked' do before do - Gitlab::LDAP::Config.any_instance.stub(block_auto_created_users: true) + allow_any_instance_of(Gitlab::LDAP::Config). + to receive(:block_auto_created_users).and_return(true) end it "does not unblock user in GitLab" do @@ -51,7 +61,8 @@ describe Gitlab::LDAP::Access do context "when auto-created users are not blocked" do before do - Gitlab::LDAP::Config.any_instance.stub(block_auto_created_users: false) + allow_any_instance_of(Gitlab::LDAP::Config). + to receive(:block_auto_created_users).and_return(false) end it "should unblock user in GitLab" do @@ -63,8 +74,9 @@ describe Gitlab::LDAP::Access do context 'without ActiveDirectory enabled' do before do - Gitlab::LDAP::Config.stub(enabled?: true) - Gitlab::LDAP::Config.any_instance.stub(active_directory: false) + allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true) + allow_any_instance_of(Gitlab::LDAP::Config). + to receive(:active_directory).and_return(false) end it { is_expected.to be_truthy } diff --git a/spec/lib/gitlab/ldap/adapter_spec.rb b/spec/lib/gitlab/ldap/adapter_spec.rb index b609e4b38f2..38076602df9 100644 --- a/spec/lib/gitlab/ldap/adapter_spec.rb +++ b/spec/lib/gitlab/ldap/adapter_spec.rb @@ -3,27 +3,32 @@ require 'spec_helper' describe Gitlab::LDAP::Adapter do let(:adapter) { Gitlab::LDAP::Adapter.new 'ldapmain' } - describe :dn_matches_filter? do + describe '#dn_matches_filter?' do let(:ldap) { double(:ldap) } subject { adapter.dn_matches_filter?(:dn, :filter) } - before { adapter.stub(ldap: ldap) } + before { allow(adapter).to receive(:ldap).and_return(ldap) } context "when the search is successful" do context "and the result is non-empty" do - before { ldap.stub(search: [:foo]) } + before { allow(ldap).to receive(:search).and_return([:foo]) } it { is_expected.to be_truthy } end context "and the result is empty" do - before { ldap.stub(search: []) } + before { allow(ldap).to receive(:search).and_return([]) } it { is_expected.to be_falsey } end end context "when the search encounters an error" do - before { ldap.stub(search: nil, get_operation_result: double(code: 1, message: 'some error')) } + before do + allow(ldap).to receive_messages( + search: nil, + get_operation_result: double(code: 1, message: 'some error') + ) + end it { is_expected.to be_falsey } end diff --git a/spec/lib/gitlab/ldap/authentication_spec.rb b/spec/lib/gitlab/ldap/authentication_spec.rb index e98b5955321..6e3de914a45 100644 --- a/spec/lib/gitlab/ldap/authentication_spec.rb +++ b/spec/lib/gitlab/ldap/authentication_spec.rb @@ -1,53 +1,58 @@ require 'spec_helper' describe Gitlab::LDAP::Authentication do - let(:klass) { Gitlab::LDAP::Authentication } - let(:user) { create(:omniauth_user, extern_uid: dn) } - let(:dn) { 'uid=john,ou=people,dc=example,dc=com' } - let(:login) { 'john' } + let(:user) { create(:omniauth_user, extern_uid: dn) } + let(:dn) { 'uid=john,ou=people,dc=example,dc=com' } + let(:login) { 'john' } let(:password) { 'password' } - describe :login do - let(:adapter) { double :adapter } + describe 'login' do before do - Gitlab::LDAP::Config.stub(enabled?: true) + allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true) end it "finds the user if authentication is successful" do - user + expect(user).not_to be_nil + # try only to fake the LDAP call - klass.any_instance.stub(adapter: double(:adapter, - bind_as: double(:ldap_user, dn: dn) - )) - expect(klass.login(login, password)).to be_truthy + adapter = double('adapter', dn: dn).as_null_object + allow_any_instance_of(described_class). + to receive(:adapter).and_return(adapter) + + expect(described_class.login(login, password)).to be_truthy end it "is false if the user does not exist" do # try only to fake the LDAP call - klass.any_instance.stub(adapter: double(:adapter, - bind_as: double(:ldap_user, dn: dn) - )) - expect(klass.login(login, password)).to be_falsey + adapter = double('adapter', dn: dn).as_null_object + allow_any_instance_of(described_class). + to receive(:adapter).and_return(adapter) + + expect(described_class.login(login, password)).to be_falsey end it "is false if authentication fails" do - user + expect(user).not_to be_nil + # try only to fake the LDAP call - klass.any_instance.stub(adapter: double(:adapter, bind_as: nil)) - expect(klass.login(login, password)).to be_falsey + adapter = double('adapter', bind_as: nil).as_null_object + allow_any_instance_of(described_class). + to receive(:adapter).and_return(adapter) + + expect(described_class.login(login, password)).to be_falsey end it "fails if ldap is disabled" do - Gitlab::LDAP::Config.stub(enabled?: false) - expect(klass.login(login, password)).to be_falsey + allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(false) + expect(described_class.login(login, password)).to be_falsey end it "fails if no login is supplied" do - expect(klass.login('', password)).to be_falsey + expect(described_class.login('', password)).to be_falsey end it "fails if no password is supplied" do - expect(klass.login(login, '')).to be_falsey + expect(described_class.login(login, '')).to be_falsey end end end diff --git a/spec/lib/gitlab/ldap/config_spec.rb b/spec/lib/gitlab/ldap/config_spec.rb index 00e9076c787..3548d647c84 100644 --- a/spec/lib/gitlab/ldap/config_spec.rb +++ b/spec/lib/gitlab/ldap/config_spec.rb @@ -14,7 +14,7 @@ describe Gitlab::LDAP::Config do end it "raises an error if a unknow provider is used" do - expect{ Gitlab::LDAP::Config.new 'unknown' }.to raise_error + expect{ Gitlab::LDAP::Config.new 'unknown' }.to raise_error(RuntimeError) end end end diff --git a/spec/lib/gitlab/ldap/user_spec.rb b/spec/lib/gitlab/ldap/user_spec.rb index 42015c28c81..7cfca96f4e0 100644 --- a/spec/lib/gitlab/ldap/user_spec.rb +++ b/spec/lib/gitlab/ldap/user_spec.rb @@ -16,31 +16,31 @@ describe Gitlab::LDAP::User do describe :changed? do it "marks existing ldap user as changed" do - existing_user = create(:omniauth_user, extern_uid: 'my-uid', provider: 'ldapmain') + create(:omniauth_user, extern_uid: 'my-uid', provider: 'ldapmain') expect(ldap_user.changed?).to be_truthy end it "marks existing non-ldap user if the email matches as changed" do - existing_user = create(:user, email: 'john@example.com') + create(:user, email: 'john@example.com') expect(ldap_user.changed?).to be_truthy end it "dont marks existing ldap user as changed" do - existing_user = create(:omniauth_user, email: 'john@example.com', extern_uid: 'my-uid', provider: 'ldapmain') + create(:omniauth_user, email: 'john@example.com', extern_uid: 'my-uid', provider: 'ldapmain') expect(ldap_user.changed?).to be_falsey end end describe :find_or_create do it "finds the user if already existing" do - existing_user = create(:omniauth_user, extern_uid: 'my-uid', provider: 'ldapmain') + create(:omniauth_user, extern_uid: 'my-uid', provider: 'ldapmain') - expect{ ldap_user.save }.to_not change{ User.count } + expect{ ldap_user.save }.not_to change{ User.count } end it "connects to existing non-ldap user if the email matches" do existing_user = create(:omniauth_user, email: 'john@example.com', provider: "twitter") - expect{ ldap_user.save }.to_not change{ User.count } + expect{ ldap_user.save }.not_to change{ User.count } existing_user.reload expect(existing_user.ldap_identity.extern_uid).to eql 'my-uid' @@ -52,11 +52,15 @@ describe Gitlab::LDAP::User do end end - describe 'blocking' do + def configure_block(value) + allow_any_instance_of(Gitlab::LDAP::Config). + to receive(:block_auto_created_users).and_return(value) + end + context 'signup' do context 'dont block on create' do - before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: false } + before { configure_block(false) } it do ldap_user.save @@ -66,7 +70,7 @@ describe Gitlab::LDAP::User do end context 'block on create' do - before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: true } + before { configure_block(true) } it do ldap_user.save @@ -83,7 +87,7 @@ describe Gitlab::LDAP::User do end context 'dont block on create' do - before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: false } + before { configure_block(false) } it do ldap_user.save @@ -93,7 +97,7 @@ describe Gitlab::LDAP::User do end context 'block on create' do - before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: true } + before { configure_block(true) } it do ldap_user.save diff --git a/spec/lib/gitlab/o_auth/auth_hash_spec.rb b/spec/lib/gitlab/o_auth/auth_hash_spec.rb index d957fe82a45..4c0a4a49d2a 100644 --- a/spec/lib/gitlab/o_auth/auth_hash_spec.rb +++ b/spec/lib/gitlab/o_auth/auth_hash_spec.rb @@ -51,7 +51,7 @@ describe Gitlab::OAuth::AuthHash do it { expect(auth_hash.email).to eql email_utf8 } it { expect(auth_hash.username).to eql nickname_utf8 } it { expect(auth_hash.name).to eql name_utf8 } - it { expect(auth_hash.password).to_not be_empty } + it { expect(auth_hash.password).not_to be_empty } end context 'email not provided' do diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb index d383ea2d051..c6cca98a037 100644 --- a/spec/lib/gitlab/o_auth/user_spec.rb +++ b/spec/lib/gitlab/o_auth/user_spec.rb @@ -19,23 +19,34 @@ describe Gitlab::OAuth::User do let!(:existing_user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'my-provider') } it "finds an existing user based on uid and provider (facebook)" do + # FIXME (rspeicher): It's unlikely that this test is actually doing anything + # `auth` is never used and removing it entirely doesn't break the test, so + # what's it doing? auth = double(info: double(name: 'John'), uid: 'my-uid', provider: 'my-provider') expect( oauth_user.persisted? ).to be_truthy end it "returns false if use is not found in database" do - auth_hash.stub(uid: 'non-existing') + allow(auth_hash).to receive(:uid).and_return('non-existing') expect( oauth_user.persisted? ).to be_falsey end end describe :save do + def stub_omniauth_config(messages) + allow(Gitlab.config.omniauth).to receive_messages(messages) + end + + def stub_ldap_config(messages) + allow(Gitlab::LDAP::Config).to receive_messages(messages) + end + let(:provider) { 'twitter' } describe 'signup' do shared_examples "to verify compliance with allow_single_sign_on" do context "with allow_single_sign_on enabled" do - before { Gitlab.config.omniauth.stub allow_single_sign_on: true } + before { stub_omniauth_config(allow_single_sign_on: true) } it "creates a user from Omniauth" do oauth_user.save @@ -48,7 +59,7 @@ describe Gitlab::OAuth::User do end context "with allow_single_sign_on disabled (Default)" do - before { Gitlab.config.omniauth.stub allow_single_sign_on: false } + before { stub_omniauth_config(allow_single_sign_on: false) } it "throws an error" do expect{ oauth_user.save }.to raise_error StandardError end @@ -56,36 +67,36 @@ describe Gitlab::OAuth::User do end context "with auto_link_ldap_user disabled (default)" do - before { Gitlab.config.omniauth.stub auto_link_ldap_user: false } + before { stub_omniauth_config(auto_link_ldap_user: false) } include_examples "to verify compliance with allow_single_sign_on" end context "with auto_link_ldap_user enabled" do - before { Gitlab.config.omniauth.stub auto_link_ldap_user: true } - + before { stub_omniauth_config(auto_link_ldap_user: true) } + context "and no LDAP provider defined" do - before { allow(Gitlab::LDAP::Config).to receive(:providers).and_return([]) } - + before { stub_ldap_config(providers: []) } + include_examples "to verify compliance with allow_single_sign_on" end - + context "and at least one LDAP provider is defined" do - before { allow(Gitlab::LDAP::Config).to receive(:providers).and_return(['ldapmain']) } + before { stub_ldap_config(providers: %w(ldapmain)) } context "and a corresponding LDAP person" do before do - ldap_user.stub(:uid) { uid } - ldap_user.stub(:username) { uid } - ldap_user.stub(:email) { ['johndoe@example.com','john2@example.com'] } - ldap_user.stub(:dn) { 'uid=user1,ou=People,dc=example' } + allow(ldap_user).to receive(:uid) { uid } + allow(ldap_user).to receive(:username) { uid } + allow(ldap_user).to receive(:email) { ['johndoe@example.com','john2@example.com'] } + allow(ldap_user).to receive(:dn) { 'uid=user1,ou=People,dc=example' } allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user) end - + context "and no account for the LDAP user" do - + it "creates a user with dual LDAP and omniauth identities" do oauth_user.save - + expect(gl_user).to be_valid expect(gl_user.username).to eql uid expect(gl_user.email).to eql 'johndoe@example.com' @@ -97,12 +108,12 @@ describe Gitlab::OAuth::User do ]) end end - + context "and LDAP user has an account already" do let!(:existing_user) { create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=user1,ou=People,dc=example', provider: 'ldapmain', username: 'john') } it "adds the omniauth identity to the LDAP account" do oauth_user.save - + expect(gl_user).to be_valid expect(gl_user.username).to eql 'john' expect(gl_user.email).to eql 'john@example.com' @@ -115,10 +126,10 @@ describe Gitlab::OAuth::User do end end end - + context "and no corresponding LDAP person" do before { allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(nil) } - + include_examples "to verify compliance with allow_single_sign_on" end end @@ -128,11 +139,11 @@ describe Gitlab::OAuth::User do describe 'blocking' do let(:provider) { 'twitter' } - before { Gitlab.config.omniauth.stub allow_single_sign_on: true } + before { stub_omniauth_config(allow_single_sign_on: true) } context 'signup with omniauth only' do context 'dont block on create' do - before { Gitlab.config.omniauth.stub block_auto_created_users: false } + before { stub_omniauth_config(block_auto_created_users: false) } it do oauth_user.save @@ -142,7 +153,7 @@ describe Gitlab::OAuth::User do end context 'block on create' do - before { Gitlab.config.omniauth.stub block_auto_created_users: true } + before { stub_omniauth_config(block_auto_created_users: true) } it do oauth_user.save @@ -154,17 +165,17 @@ describe Gitlab::OAuth::User do context 'signup with linked omniauth and LDAP account' do before do - Gitlab.config.omniauth.stub auto_link_ldap_user: true - ldap_user.stub(:uid) { uid } - ldap_user.stub(:username) { uid } - ldap_user.stub(:email) { ['johndoe@example.com','john2@example.com'] } - ldap_user.stub(:dn) { 'uid=user1,ou=People,dc=example' } + stub_omniauth_config(auto_link_ldap_user: true) + allow(ldap_user).to receive(:uid) { uid } + allow(ldap_user).to receive(:username) { uid } + allow(ldap_user).to receive(:email) { ['johndoe@example.com','john2@example.com'] } + allow(ldap_user).to receive(:dn) { 'uid=user1,ou=People,dc=example' } allow(oauth_user).to receive(:ldap_person).and_return(ldap_user) end context "and no account for the LDAP user" do context 'dont block on create (LDAP)' do - before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: false } + before { allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(block_auto_created_users: false) } it do oauth_user.save @@ -174,7 +185,7 @@ describe Gitlab::OAuth::User do end context 'block on create (LDAP)' do - before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: true } + before { allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(block_auto_created_users: true) } it do oauth_user.save @@ -188,7 +199,7 @@ describe Gitlab::OAuth::User do let!(:existing_user) { create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=user1,ou=People,dc=example', provider: 'ldapmain', username: 'john') } context 'dont block on create (LDAP)' do - before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: false } + before { allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(block_auto_created_users: false) } it do oauth_user.save @@ -198,7 +209,7 @@ describe Gitlab::OAuth::User do end context 'block on create (LDAP)' do - before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: true } + before { allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(block_auto_created_users: true) } it do oauth_user.save @@ -217,7 +228,7 @@ describe Gitlab::OAuth::User do end context 'dont block on create' do - before { Gitlab.config.omniauth.stub block_auto_created_users: false } + before { stub_omniauth_config(block_auto_created_users: false) } it do oauth_user.save @@ -227,7 +238,7 @@ describe Gitlab::OAuth::User do end context 'block on create' do - before { Gitlab.config.omniauth.stub block_auto_created_users: true } + before { stub_omniauth_config(block_auto_created_users: true) } it do oauth_user.save @@ -237,7 +248,7 @@ describe Gitlab::OAuth::User do end context 'dont block on create (LDAP)' do - before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: false } + before { allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(block_auto_created_users: false) } it do oauth_user.save @@ -247,7 +258,7 @@ describe Gitlab::OAuth::User do end context 'block on create (LDAP)' do - before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: true } + before { allow_any_instance_of(Gitlab::LDAP::Config).to receive_messages(block_auto_created_users: true) } it do oauth_user.save diff --git a/spec/lib/gitlab/popen_spec.rb b/spec/lib/gitlab/popen_spec.rb index 0bc1d5630e6..e53efec6c67 100644 --- a/spec/lib/gitlab/popen_spec.rb +++ b/spec/lib/gitlab/popen_spec.rb @@ -28,7 +28,7 @@ describe 'Gitlab::Popen', no_db: true do context 'unsafe string command' do it 'raises an error when it gets called with a string argument' do - expect { @klass.new.popen('ls', path) }.to raise_error + expect { @klass.new.popen('ls', path) }.to raise_error(RuntimeError) end end diff --git a/spec/lib/gitlab/satellite/merge_action_spec.rb b/spec/lib/gitlab/satellite/merge_action_spec.rb index 27912573dce..9b1c9a34e29 100644 --- a/spec/lib/gitlab/satellite/merge_action_spec.rb +++ b/spec/lib/gitlab/satellite/merge_action_spec.rb @@ -27,7 +27,7 @@ describe 'Gitlab::Satellite::MergeAction' do context 'between branches' do it 'should raise exception -- not expected to be used by non forks' do - expect { Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).commits_between }.to raise_error + expect { Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).commits_between }.to raise_error(RuntimeError) end end end @@ -75,7 +75,7 @@ describe 'Gitlab::Satellite::MergeAction' do context 'between branches' do it 'should get proper diffs' do - expect{ Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).diffs_between_satellite }.to raise_error + expect{ Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).diffs_between_satellite }.to raise_error(RuntimeError) end end end diff --git a/spec/lib/gitlab/upgrader_spec.rb b/spec/lib/gitlab/upgrader_spec.rb index baa4bd0f28f..8df84665e16 100644 --- a/spec/lib/gitlab/upgrader_spec.rb +++ b/spec/lib/gitlab/upgrader_spec.rb @@ -10,14 +10,14 @@ describe Gitlab::Upgrader do describe 'latest_version?' do it 'should be true if newest version' do - upgrader.stub(latest_version_raw: current_version) + allow(upgrader).to receive(:latest_version_raw).and_return(current_version) expect(upgrader.latest_version?).to be_truthy end end describe 'latest_version_raw' do it 'should be latest version for GitLab 5' do - upgrader.stub(current_version_raw: "5.3.0") + allow(upgrader).to receive(:current_version_raw).and_return("5.3.0") expect(upgrader.latest_version_raw).to eq("v5.4.2") end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index c1d82c5367d..97c07ad7d55 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -1,4 +1,5 @@ require 'spec_helper' +require 'email_spec' describe Notify do include EmailSpec::Helpers diff --git a/spec/models/commit_range_spec.rb b/spec/models/commit_range_spec.rb index 5e7efadde28..1031af097bd 100644 --- a/spec/models/commit_range_spec.rb +++ b/spec/models/commit_range_spec.rb @@ -14,7 +14,7 @@ describe CommitRange do let(:range2) { described_class.new("#{sha_from}..#{sha_to}") } it 'raises ArgumentError when given an invalid range string' do - expect { described_class.new("Foo") }.to raise_error + expect { described_class.new("Foo") }.to raise_error(ArgumentError) end describe '#to_s' do diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 27eb02a870b..e303a97e6b5 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -77,13 +77,13 @@ eos let(:other_issue) { create :issue, project: other_project } it 'detects issues that this commit is marked as closing' do - commit.stub(safe_message: "Fixes ##{issue.iid}") + allow(commit).to receive(:safe_message).and_return("Fixes ##{issue.iid}") expect(commit.closes_issues).to eq([issue]) end it 'does not detect issues from other projects' do ext_ref = "#{other_project.path_with_namespace}##{other_issue.iid}" - commit.stub(safe_message: "Fixes #{ext_ref}") + allow(commit).to receive(:safe_message).and_return("Fixes #{ext_ref}") expect(commit.closes_issues).to be_empty end end @@ -93,7 +93,9 @@ eos let(:author) { create(:user, email: commit.author_email) } let(:backref_text) { "commit #{subject.id}" } - let(:set_mentionable_text) { ->(txt){ subject.stub(safe_message: txt) } } + let(:set_mentionable_text) do + ->(txt) { allow(subject).to receive(:safe_message).and_return(txt) } + end # Include the subject in the repository stub. let(:extra_commits) { [subject] } diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 86c395a8e8e..b6d80451d2e 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -11,7 +11,10 @@ describe Issue, "Issuable" do end describe "Validation" do - before { subject.stub(set_iid: false) } + before do + allow(subject).to receive(:set_iid).and_return(false) + end + it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:iid) } it { is_expected.to validate_presence_of(:author) } diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 22237f2e9f2..f7f66987b5f 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -23,7 +23,7 @@ describe Issue, "Mentionable" do end it 'correctly removes already-mentioned Commits' do - expect(Note).not_to receive(:create_cross_reference_note) + expect(SystemNoteService).not_to receive(:cross_reference) issue.create_cross_references!(project, author, [commit2]) end diff --git a/spec/models/forked_project_link_spec.rb b/spec/models/forked_project_link_spec.rb index a518c42f491..d90fbfe1ea5 100644 --- a/spec/models/forked_project_link_spec.rb +++ b/spec/models/forked_project_link_spec.rb @@ -58,9 +58,10 @@ describe :forked_from_project do end def fork_project(from_project, user) - context = Projects::ForkService.new(from_project, user) - shell = double("gitlab_shell") - shell.stub(fork_repository: true) - context.stub(gitlab_shell: shell) - context.execute + shell = double('gitlab_shell', fork_repository: true) + + service = Projects::ForkService.new(from_project, user) + allow(service).to receive(:gitlab_shell).and_return(shell) + + service.execute end diff --git a/spec/models/hooks/service_hook_spec.rb b/spec/models/hooks/service_hook_spec.rb index 98cc0ede42e..4c8b8910ae7 100644 --- a/spec/models/hooks/service_hook_spec.rb +++ b/spec/models/hooks/service_hook_spec.rb @@ -50,7 +50,7 @@ describe ServiceHook do it "catches exceptions" do expect(WebHook).to receive(:post).and_raise("Some HTTP Post error") - expect { @service_hook.execute(@data) }.to raise_error + expect { @service_hook.execute(@data) }.to raise_error(RuntimeError) end end end diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index c74432d9739..23f30881d99 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -71,7 +71,7 @@ describe ProjectHook do it "catches exceptions" do expect(WebHook).to receive(:post).and_raise("Some HTTP Post error") - expect { @project_hook.execute(@data, 'push_hooks') }.to raise_error + expect { @project_hook.execute(@data, 'push_hooks') }.to raise_error(RuntimeError) end end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 614b648bb52..9bac451c28c 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -70,7 +70,7 @@ describe Issue do it_behaves_like 'an editable mentionable' do subject { create(:issue, project: project) } - let(:backref_text) { "issue ##{subject.iid}" } + let(:backref_text) { "issue #{subject.to_reference}" } let(:set_mentionable_text) { ->(txt){ subject.description = txt } } end diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 7c10c9f0f48..652026729bb 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -24,8 +24,11 @@ describe GroupMember do describe "#after_create" do it "should send email to user" do membership = build(:group_member) - membership.stub(notification_service: double('NotificationService').as_null_object) + + allow(membership).to receive(:notification_service). + and_return(double('NotificationService').as_null_object) expect(membership).to receive(:notification_service) + membership.save end end @@ -33,7 +36,8 @@ describe GroupMember do describe "#after_update" do before do @group_member = create :group_member - @group_member.stub(notification_service: double('NotificationService').as_null_object) + allow(@group_member).to receive(:notification_service). + and_return(double('NotificationService').as_null_object) end it "should send email to user" do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 0465aa34843..76f6d8c54c4 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -111,17 +111,18 @@ describe MergeRequest do let(:commit2) { double('commit2', closes_issues: [issue1]) } before do - subject.stub(commits: [commit0, commit1, commit2]) + allow(subject).to receive(:commits).and_return([commit0, commit1, commit2]) end it 'accesses the set of issues that will be closed on acceptance' do - subject.project.stub(default_branch: subject.target_branch) + allow(subject.project).to receive(:default_branch). + and_return(subject.target_branch) expect(subject.closes_issues).to eq([issue0, issue1].sort_by(&:id)) end it 'only lists issues as to be closed if it targets the default branch' do - subject.project.stub(default_branch: 'master') + allow(subject.project).to receive(:default_branch).and_return('master') subject.target_branch = 'something-else' expect(subject.closes_issues).to be_empty @@ -130,7 +131,8 @@ describe MergeRequest do it 'detects issues mentioned in the description' do issue2 = create(:issue, project: subject.project) subject.description = "Closes #{issue2.to_reference}" - subject.project.stub(default_branch: subject.target_branch) + allow(subject.project).to receive(:default_branch). + and_return(subject.target_branch) expect(subject.closes_issues).to include(issue2) end @@ -163,10 +165,10 @@ describe MergeRequest do end it_behaves_like 'an editable mentionable' do - subject { create(:merge_request, source_project: project, target_project: project) } + subject { create(:merge_request, source_project: project) } - let(:backref_text) { "merge request !#{subject.iid}" } - let(:set_mentionable_text) { ->(txt){ subject.title = txt } } + let(:backref_text) { "merge request #{subject.to_reference}" } + let(:set_mentionable_text) { ->(txt){ subject.description = txt } } end it_behaves_like 'a Taskable' do diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index eb73aa763fc..36352e1ecce 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -21,11 +21,11 @@ describe Milestone do it { is_expected.to have_many(:issues) } end - describe "Mass assignment" do - end - describe "Validation" do - before { subject.stub(set_iid: false) } + before do + allow(subject).to receive(:set_iid).and_return(false) + end + it { is_expected.to validate_presence_of(:title) } it { is_expected.to validate_presence_of(:project) } end @@ -66,7 +66,7 @@ describe Milestone do describe :expired? do context "expired" do before do - milestone.stub(due_date: Date.today.prev_year) + allow(milestone).to receive(:due_date).and_return(Date.today.prev_year) end it { expect(milestone.expired?).to be_truthy } @@ -74,7 +74,7 @@ describe Milestone do context "not expired" do before do - milestone.stub(due_date: Date.today.next_year) + allow(milestone).to receive(:due_date).and_return(Date.today.next_year) end it { expect(milestone.expired?).to be_falsey } @@ -83,7 +83,7 @@ describe Milestone do describe :percent_complete do before do - milestone.stub( + allow(milestone).to receive_messages( closed_items_count: 3, total_items_count: 4 ) diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index e87432fdf62..1d72a9503ae 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -53,7 +53,7 @@ describe Namespace do describe :move_dir do before do @namespace = create :namespace - @namespace.stub(path_changed?: true) + allow(@namespace).to receive(:path_changed?).and_return(true) end it "should raise error when directory exists" do @@ -62,8 +62,8 @@ describe Namespace do it "should move dir if path changed" do new_path = @namespace.path + "_new" - @namespace.stub(path_was: @namespace.path) - @namespace.stub(path: new_path) + allow(@namespace).to receive(:path_was).and_return(@namespace.path) + allow(@namespace).to receive(:path).and_return(new_path) expect(@namespace.move_dir).to be_truthy end end diff --git a/spec/models/project_services/asana_service_spec.rb b/spec/models/project_services/asana_service_spec.rb index cc1f99e0c72..64bb92fba95 100644 --- a/spec/models/project_services/asana_service_spec.rb +++ b/spec/models/project_services/asana_service_spec.rb @@ -42,7 +42,7 @@ describe AsanaService, models: true do before do @asana = AsanaService.new - @asana.stub( + allow(@asana).to receive_messages( project: project, project_id: project.id, service_hook: true, diff --git a/spec/models/project_services/assembla_service_spec.rb b/spec/models/project_services/assembla_service_spec.rb index 9aee754dd63..17e9361dd5c 100644 --- a/spec/models/project_services/assembla_service_spec.rb +++ b/spec/models/project_services/assembla_service_spec.rb @@ -32,7 +32,7 @@ describe AssemblaService, models: true do before do @assembla_service = AssemblaService.new - @assembla_service.stub( + allow(@assembla_service).to receive_messages( project_id: project.id, project: project, service_hook: true, diff --git a/spec/models/project_services/buildkite_service_spec.rb b/spec/models/project_services/buildkite_service_spec.rb index 6db54243eac..9445d96c337 100644 --- a/spec/models/project_services/buildkite_service_spec.rb +++ b/spec/models/project_services/buildkite_service_spec.rb @@ -29,12 +29,10 @@ describe BuildkiteService do describe 'commits methods' do before do @project = Project.new - @project.stub( - default_branch: 'default-brancho' - ) + allow(@project).to receive(:default_branch).and_return('default-brancho') @service = BuildkiteService.new - @service.stub( + allow(@service).to receive_messages( project: @project, service_hook: true, project_url: 'https://buildkite.com/account-name/example-project', diff --git a/spec/models/project_services/flowdock_service_spec.rb b/spec/models/project_services/flowdock_service_spec.rb index e6e8fbba6a7..7e5b15cb09e 100644 --- a/spec/models/project_services/flowdock_service_spec.rb +++ b/spec/models/project_services/flowdock_service_spec.rb @@ -32,7 +32,7 @@ describe FlowdockService do before do @flowdock_service = FlowdockService.new - @flowdock_service.stub( + allow(@flowdock_service).to receive_messages( project_id: project.id, project: project, service_hook: true, diff --git a/spec/models/project_services/gemnasium_service_spec.rb b/spec/models/project_services/gemnasium_service_spec.rb index 1a7765e5c2a..9e156472316 100644 --- a/spec/models/project_services/gemnasium_service_spec.rb +++ b/spec/models/project_services/gemnasium_service_spec.rb @@ -32,7 +32,7 @@ describe GemnasiumService do before do @gemnasium_service = GemnasiumService.new - @gemnasium_service.stub( + allow(@gemnasium_service).to receive_messages( project_id: project.id, project: project, service_hook: true, diff --git a/spec/models/project_services/gitlab_ci_service_spec.rb b/spec/models/project_services/gitlab_ci_service_spec.rb index c92cf3cdae6..fedc37c9b94 100644 --- a/spec/models/project_services/gitlab_ci_service_spec.rb +++ b/spec/models/project_services/gitlab_ci_service_spec.rb @@ -21,18 +21,15 @@ require 'spec_helper' describe GitlabCiService do - describe "Associations" do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - - describe "Mass assignment" do + describe 'associations' do + it { is_expected.to belong_to(:project) } + it { is_expected.to have_one(:service_hook) } end describe 'commits methods' do before do @service = GitlabCiService.new - @service.stub( + allow(@service).to receive_messages( service_hook: true, project_url: 'http://ci.gitlab.org/projects/2', token: 'verySecret' @@ -56,9 +53,9 @@ describe GitlabCiService do it "calls ci_yaml_file" do service_hook = double - service_hook.should_receive(:execute) - @service.should_receive(:service_hook).and_return(service_hook) - @service.should_receive(:ci_yaml_file).with(push_sample_data[:checkout_sha]) + expect(service_hook).to receive(:execute) + expect(@service).to receive(:service_hook).and_return(service_hook) + expect(@service).to receive(:ci_yaml_file).with(push_sample_data[:checkout_sha]) @service.execute(push_sample_data) end @@ -72,7 +69,7 @@ describe GitlabCiService do @user = create(:user) @service = GitlabCiService.new - @service.stub( + allow(@service).to receive_messages( service_hook: true, project_url: 'http://ci.gitlab.org/projects/2', token: 'verySecret', diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index 8e2fcca2657..4707673269a 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -37,7 +37,7 @@ describe HipchatService do let(:push_sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) } before(:each) do - hipchat.stub( + allow(hipchat).to receive_messages( project_id: project.id, project: project, room: 123456, @@ -48,7 +48,7 @@ describe HipchatService do end it 'should use v1 if version is provided' do - hipchat.stub(api_version: 'v1') + allow(hipchat).to receive(:api_version).and_return('v1') expect(HipChat::Client).to receive(:new). with(token, api_version: 'v1', @@ -59,7 +59,7 @@ describe HipchatService do end it 'should use v2 as the version when nothing is provided' do - hipchat.stub(api_version: '') + allow(hipchat).to receive(:api_version).and_return('') expect(HipChat::Client).to receive(:new). with(token, api_version: 'v2', @@ -245,12 +245,12 @@ describe HipchatService do end it "should set notfiy to true" do - hipchat.stub(notify: '1') + allow(hipchat).to receive(:notify).and_return('1') expect(hipchat.send(:message_options)).to eq({ notify: true, color: 'yellow' }) end it "should set the color" do - hipchat.stub(color: 'red') + allow(hipchat).to receive(:color).and_return('red') expect(hipchat.send(:message_options)).to eq({ notify: false, color: 'red' }) end end diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb index 4c437aab12b..37690434ec8 100644 --- a/spec/models/project_services/irker_service_spec.rb +++ b/spec/models/project_services/irker_service_spec.rb @@ -24,8 +24,8 @@ require 'json' describe IrkerService do describe 'Associations' do - it { should belong_to :project } - it { should have_one :service_hook } + it { is_expected.to belong_to :project } + it { is_expected.to have_one :service_hook } end describe 'Validations' do @@ -66,7 +66,7 @@ describe IrkerService do let(:colorize_messages) { '1' } before do - irker.stub( + allow(irker).to receive_messages( active: true, project: project, project_id: project.id, diff --git a/spec/models/project_services/pushover_service_spec.rb b/spec/models/project_services/pushover_service_spec.rb index 5f93703b50a..ac10ffbd39b 100644 --- a/spec/models/project_services/pushover_service_spec.rb +++ b/spec/models/project_services/pushover_service_spec.rb @@ -52,7 +52,7 @@ describe PushoverService do let(:api_url) { 'https://api.pushover.net/1/messages.json' } before do - pushover.stub( + allow(pushover).to receive_messages( project: project, project_id: project.id, service_hook: true, diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb index e9105677d23..69466b11f09 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/project_services/slack_service_spec.rb @@ -46,7 +46,7 @@ describe SlackService do let(:channel) { 'slack_channel' } before do - slack.stub( + allow(slack).to receive_messages( project: project, project_id: project.id, service_hook: true, @@ -96,7 +96,7 @@ describe SlackService do end it 'should use the username as an option for slack when configured' do - slack.stub(username: username) + allow(slack).to receive(:username).and_return(username) expect(Slack::Notifier).to receive(:new). with(webhook_url, username: username). and_return( @@ -106,7 +106,7 @@ describe SlackService do end it 'should use the channel as an option when it is configured' do - slack.stub(channel: channel) + allow(slack).to receive(:channel).and_return(channel) expect(Slack::Notifier).to receive(:new). with(webhook_url, channel: channel). and_return( @@ -130,11 +130,11 @@ describe SlackService do let(:webhook_url) { 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' } before do - slack.stub( - project: project, - project_id: project.id, - service_hook: true, - webhook: webhook_url + allow(slack).to receive_messages( + project: project, + project_id: project.id, + service_hook: true, + webhook: webhook_url ) WebMock.stub_request(:post, webhook_url) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 87c67fa32c3..63091e913ff 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -127,7 +127,7 @@ describe Project do describe 'last_activity' do it 'should alias last_activity to last_event' do - project.stub(last_event: last_event) + allow(project).to receive(:last_event).and_return(last_event) expect(project.last_activity).to eq(last_event) end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 869ace885f0..ca11758ee06 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -39,9 +39,7 @@ describe Service do let(:project) { create :project } before do - @service.stub( - project: project - ) + allow(@service).to receive(:project).and_return(project) @testable = @service.can_test? end @@ -54,9 +52,7 @@ describe Service do let(:project) { create :project } before do - @service.stub( - project: project - ) + allow(@service).to receive(:project).and_return(project) @testable = @service.can_test? end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index afdd9f71af2..9f7c83f3476 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -458,21 +458,25 @@ describe User do it 'is false when LDAP is disabled' do # Create a condition which would otherwise cause 'true' to be returned - user.stub(ldap_user?: true) + allow(user).to receive(:ldap_user?).and_return(true) user.last_credential_check_at = nil expect(user.requires_ldap_check?).to be_falsey end context 'when LDAP is enabled' do - before { Gitlab.config.ldap.stub(enabled: true) } + before do + allow(Gitlab.config.ldap).to receive(:enabled).and_return(true) + end it 'is false for non-LDAP users' do - user.stub(ldap_user?: false) + allow(user).to receive(:ldap_user?).and_return(false) expect(user.requires_ldap_check?).to be_falsey end context 'and when the user is an LDAP user' do - before { user.stub(ldap_user?: true) } + before do + allow(user).to receive(:ldap_user?).and_return(true) + end it 'is true when the user has never had an LDAP check before' do user.last_credential_check_at = nil diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb new file mode 100644 index 00000000000..671fd6c8666 --- /dev/null +++ b/spec/rails_helper.rb @@ -0,0 +1 @@ +require "spec_helper" diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb index 20cb30a39bb..4048c297013 100644 --- a/spec/requests/api/api_helpers_spec.rb +++ b/spec/requests/api/api_helpers_spec.rb @@ -47,7 +47,7 @@ describe API, api: true do it "should return nil for a user without access" do env[API::APIHelpers::PRIVATE_TOKEN_HEADER] = user.private_token - Gitlab::UserAccess.stub(allowed?: false) + allow(Gitlab::UserAccess).to receive(:allowed?).and_return(false) expect(current_user).to be_nil end @@ -72,13 +72,13 @@ describe API, api: true do it "should throw an error when the current user is not an admin and attempting to sudo" do set_env(user, admin.id) - expect { current_user }.to raise_error + expect { current_user }.to raise_error(Exception) set_param(user, admin.id) - expect { current_user }.to raise_error + expect { current_user }.to raise_error(Exception) set_env(user, admin.username) - expect { current_user }.to raise_error + expect { current_user }.to raise_error(Exception) set_param(user, admin.username) - expect { current_user }.to raise_error + expect { current_user }.to raise_error(Exception) end it "should throw an error when the user cannot be found for a given id" do @@ -86,10 +86,10 @@ describe API, api: true do expect(user.id).not_to eq(id) expect(admin.id).not_to eq(id) set_env(admin, id) - expect { current_user }.to raise_error + expect { current_user }.to raise_error(Exception) set_param(admin, id) - expect { current_user }.to raise_error + expect { current_user }.to raise_error(Exception) end it "should throw an error when the user cannot be found for a given username" do @@ -97,10 +97,10 @@ describe API, api: true do expect(user.username).not_to eq(username) expect(admin.username).not_to eq(username) set_env(admin, username) - expect { current_user }.to raise_error + expect { current_user }.to raise_error(Exception) set_param(admin, username) - expect { current_user }.to raise_error + expect { current_user }.to raise_error(Exception) end it "should handle sudo's to oneself" do diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index f40d68b75a4..cb6e5e89625 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -141,7 +141,9 @@ describe API::API, api: true do end describe "DELETE /projects/:id/repository/branches/:branch" do - before { Repository.any_instance.stub(rm_branch: true) } + before do + allow_any_instance_of(Repository).to receive(:rm_branch).and_return(true) + end it "should remove branch" do delete api("/projects/#{project.id}/repository/branches/#{branch_name}", user) diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 27a26b56fef..6c7860511e8 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -60,9 +60,8 @@ describe API::API, api: true do end it "should return a 400 if editor fails to create file" do - Repository.any_instance.stub( - commit_file: false, - ) + allow_any_instance_of(Repository).to receive(:commit_file). + and_return(false) post api("/projects/#{project.id}/repository/files", user), valid_params expect(response.status).to eq(400) @@ -112,9 +111,7 @@ describe API::API, api: true do end it "should return a 400 if satellite fails to create file" do - Repository.any_instance.stub( - remove_file: false, - ) + allow_any_instance_of(Repository).to receive(:remove_file).and_return(false) delete api("/projects/#{project.id}/repository/files", user), valid_params expect(response.status).to eq(400) diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index e0574151abb..c5a4ac7e4c4 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -167,7 +167,8 @@ describe API::API, api: true do describe "POST /groups/:id/projects/:project_id" do let(:project) { create(:project) } before(:each) do - Projects::TransferService.any_instance.stub(execute: true) + allow_any_instance_of(Projects::TransferService). + to receive(:execute).and_return(true) allow(Project).to receive(:find).and_return(project) end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index cb7da3aa3c4..7030c105b58 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -50,9 +50,8 @@ describe API::API, api: true do get api("/projects/#{project.id}/merge_requests?state=closed", user) expect(response.status).to eq(200) expect(json_response).to be_an Array - expect(json_response.length).to eq(2) - expect(json_response.second['title']).to eq(merge_request_closed.title) - expect(json_response.first['title']).to eq(merge_request_merged.title) + expect(json_response.length).to eq(1) + expect(json_response.first['title']).to eq(merge_request_closed.title) end it "should return an array of merged merge_requests" do @@ -302,14 +301,20 @@ describe API::API, api: true do describe "PUT /projects/:id/merge_request/:merge_request_id/merge" do it "should return merge_request in case of success" do - MergeRequest.any_instance.stub(can_be_merged?: true, automerge!: true) + allow_any_instance_of(MergeRequest). + to receive_messages(can_be_merged?: true, automerge!: true) + put api("/projects/#{project.id}/merge_request/#{merge_request.id}/merge", user) + expect(response.status).to eq(200) end it "should return 405 if branch can't be merged" do - MergeRequest.any_instance.stub(can_be_merged?: false) + allow_any_instance_of(MergeRequest). + to receive(:can_be_merged?).and_return(false) + put api("/projects/#{project.id}/merge_request/#{merge_request.id}/merge", user) + expect(response.status).to eq(405) expect(json_response['message']).to eq('Branch cannot be merged') end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 2ec8e0f354c..e9ff832603f 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -121,15 +121,13 @@ describe API::API, api: true do get api('/projects/all', admin) expect(response.status).to eq(200) expect(json_response).to be_an Array - project_name = project.name - expect(json_response.detect do |project| - project['name'] == project_name - end['name']).to eq(project_name) - - expect(json_response.detect do |project| - project['owner']['username'] == user.username - end['owner']['username']).to eq(user.username) + expect(json_response).to satisfy do |response| + response.one? do |entry| + entry['name'] == project.name && + entry['owner']['username'] == user.username + end + end end end end @@ -138,9 +136,8 @@ describe API::API, api: true do context 'maximum number of projects reached' do it 'should not create new project and respond with 403' do allow_any_instance_of(User).to receive(:projects_limit_left).and_return(0) - expect do - post api('/projects', user2), name: 'foo' - end.to change {Project.count}.by(0) + expect { post api('/projects', user2), name: 'foo' }. + to change {Project.count}.by(0) expect(response.status).to eq(403) end end @@ -158,7 +155,7 @@ describe API::API, api: true do end it 'should not create new project without name and return 400' do - expect { post api('/projects', user) }.to_not change { Project.count } + expect { post api('/projects', user) }.not_to change { Project.count } expect(response.status).to eq(400) end @@ -257,7 +254,7 @@ describe API::API, api: true do it 'should respond with 400 on failure and not project' do expect { post api("/projects/user/#{user.id}", admin) }. - to_not change { Project.count } + not_to change { Project.count } expect(response.status).to eq(400) expect(json_response['message']['name']).to eq([ diff --git a/spec/services/archive_repository_service_spec.rb b/spec/services/archive_repository_service_spec.rb index a7a783ea2e4..0ec70c51b3a 100644 --- a/spec/services/archive_repository_service_spec.rb +++ b/spec/services/archive_repository_service_spec.rb @@ -17,7 +17,7 @@ describe ArchiveRepositoryService do end it "raises an error" do - expect { subject.execute(timeout: 0.0) }.to raise_error + expect { subject.execute(timeout: 0.0) }.to raise_error(RuntimeError) end end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index b3e59d7a0db..3373b97bfd4 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -124,7 +124,9 @@ describe GitPushService do end it "when pushing a branch for the first time with default branch protection disabled" do - ApplicationSetting.any_instance.stub(default_branch_protection: 0) + allow(ApplicationSetting.current_application_settings). + to receive(:default_branch_protection). + and_return(Gitlab::Access::PROTECTION_NONE) expect(project).to receive(:execute_hooks) expect(project.default_branch).to eq("master") @@ -133,7 +135,9 @@ describe GitPushService do end it "when pushing a branch for the first time with default branch protection set to 'developers can push'" do - ApplicationSetting.any_instance.stub(default_branch_protection: 1) + allow(ApplicationSetting.current_application_settings). + to receive(:default_branch_protection). + and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH) expect(project).to receive(:execute_hooks) expect(project.default_branch).to eq("master") @@ -154,32 +158,35 @@ describe GitPushService do let(:commit) { project.commit } before do - commit.stub({ + allow(commit).to receive_messages( safe_message: "this commit \n mentions ##{issue.id}", references: [issue], author_name: commit_author.name, author_email: commit_author.email - }) - project.repository.stub(commits_between: [commit]) + ) + allow(project.repository).to receive(:commits_between).and_return([commit]) end it "creates a note if a pushed commit mentions an issue" do - expect(Note).to receive(:create_cross_reference_note).with(issue, commit, commit_author) + expect(SystemNoteService).to receive(:cross_reference).with(issue, commit, commit_author) service.execute(project, user, @oldrev, @newrev, @ref) end it "only creates a cross-reference note if one doesn't already exist" do - Note.create_cross_reference_note(issue, commit, user) + SystemNoteService.cross_reference(issue, commit, user) - expect(Note).not_to receive(:create_cross_reference_note).with(issue, commit, commit_author) + expect(SystemNoteService).not_to receive(:cross_reference).with(issue, commit, commit_author) service.execute(project, user, @oldrev, @newrev, @ref) end it "defaults to the pushing user if the commit's author is not known" do - commit.stub(author_name: 'unknown name', author_email: 'unknown@email.com') - expect(Note).to receive(:create_cross_reference_note).with(issue, commit, user) + allow(commit).to receive_messages( + author_name: 'unknown name', + author_email: 'unknown@email.com' + ) + expect(SystemNoteService).to receive(:cross_reference).with(issue, commit, user) service.execute(project, user, @oldrev, @newrev, @ref) end @@ -188,7 +195,7 @@ describe GitPushService do allow(project.repository).to receive(:commits_between).with(@blankrev, @newrev).and_return([]) allow(project.repository).to receive(:commits_between).with("master", @newrev).and_return([commit]) - expect(Note).to receive(:create_cross_reference_note).with(issue, commit, commit_author) + expect(SystemNoteService).to receive(:cross_reference).with(issue, commit, commit_author) service.execute(project, user, @blankrev, @newrev, 'refs/heads/other') end @@ -201,14 +208,15 @@ describe GitPushService do let(:closing_commit) { project.commit } before do - closing_commit.stub({ + allow(closing_commit).to receive_messages( issue_closing_regex: /^([Cc]loses|[Ff]ixes) #\d+/, safe_message: "this is some work.\n\ncloses ##{issue.iid}", author_name: commit_author.name, author_email: commit_author.email - }) + ) - project.repository.stub(commits_between: [closing_commit]) + allow(project.repository).to receive(:commits_between). + and_return([closing_commit]) end it "closes issues with commit messages" do @@ -224,7 +232,7 @@ describe GitPushService do end it "doesn't close issues when pushed to non-default branches" do - project.stub(default_branch: 'durf') + allow(project).to receive(:default_branch).and_return('durf') # The push still shouldn't create cross-reference notes. expect do diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 62a99d15952..253e5823499 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -58,7 +58,7 @@ describe NotificationService do end it 'filters out "mentioned in" notes' do - mentioned_note = Note.create_cross_reference_note(mentioned_issue, issue, issue.author) + mentioned_note = SystemNoteService.cross_reference(mentioned_issue, issue, issue.author) expect(Notify).not_to receive(:note_issue_email) notification.new_note(mentioned_note) @@ -130,7 +130,7 @@ describe NotificationService do end it 'filters out "mentioned in" notes' do - mentioned_note = Note.create_cross_reference_note(mentioned_issue, issue, issue.author) + mentioned_note = SystemNoteService.cross_reference(mentioned_issue, issue, issue.author) expect(Notify).not_to receive(:note_issue_email) notification.new_note(mentioned_note) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9c8004ab555..666d56079d7 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,7 +2,6 @@ ENV["RAILS_ENV"] ||= 'test' require File.expand_path("../../config/environment", __FILE__) require 'rspec/rails' require 'shoulda/matchers' -require 'email_spec' require 'sidekiq/testing/inline' # Requires supporting ruby files with custom matchers and macros, etc, diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index 3e41aec425a..fed1ab6ee33 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -19,36 +19,3 @@ unless ENV['CI'] || ENV['CI_SERVER'] # Keep only the screenshots generated from the last failing test suite Capybara::Screenshot.prune_strategy = :keep_last_run end - -module CapybaraHelpers - # Execute a block a certain number of times before considering it a failure - # - # The given block is called, and if it raises a `Capybara::ExpectationNotMet` - # error, we wait `interval` seconds and then try again, until `retries` is - # met. - # - # This allows for better handling of timing-sensitive expectations in a - # sketchy CI environment, for example. - # - # interval - Delay between retries in seconds (default: 0.5) - # retries - Number of times to execute before failing (default: 5) - def allowing_for_delay(interval: 0.5, retries: 5) - tries = 0 - - begin - yield - rescue Capybara::ExpectationNotMet => ex - if tries <= retries - tries += 1 - sleep interval - retry - else - raise ex - end - end - end -end - -RSpec.configure do |config| - config.include CapybaraHelpers, type: :feature -end diff --git a/spec/support/capybara_helpers.rb b/spec/support/capybara_helpers.rb new file mode 100644 index 00000000000..9b5c3065eed --- /dev/null +++ b/spec/support/capybara_helpers.rb @@ -0,0 +1,34 @@ +module CapybaraHelpers + # Execute a block a certain number of times before considering it a failure + # + # The given block is called, and if it raises a `Capybara::ExpectationNotMet` + # error, we wait `interval` seconds and then try again, until `retries` is + # met. + # + # This allows for better handling of timing-sensitive expectations in a + # sketchy CI environment, for example. + # + # interval - Delay between retries in seconds (default: 0.5) + # retries - Number of times to execute before failing (default: 5) + def allowing_for_delay(interval: 0.5, retries: 5) + tries = 0 + + begin + sleep interval + + yield + rescue Capybara::ExpectationNotMet => ex + if tries <= retries + tries += 1 + sleep interval + retry + else + raise ex + end + end + end +end + +RSpec.configure do |config| + config.include CapybaraHelpers, type: :feature +end diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb index 6250737b9cd..e0dbc9aa84c 100644 --- a/spec/support/db_cleaner.rb +++ b/spec/support/db_cleaner.rb @@ -1,21 +1,3 @@ -# RSpec.configure do |config| - -# config.around(:each) do |example| -# DatabaseCleaner.strategy = :transaction -# DatabaseCleaner.clean_with(:truncation) -# DatabaseCleaner.cleaning do -# example.run -# end -# end - -# config.around(:each, js: true) do |example| -# DatabaseCleaner.strategy = :truncation -# DatabaseCleaner.clean_with(:truncation) -# DatabaseCleaner.cleaning do -# example.run -# end -# end -# end RSpec.configure do |config| config.before(:suite) do DatabaseCleaner.clean_with(:truncation) @@ -36,15 +18,4 @@ RSpec.configure do |config| config.after(:each) do DatabaseCleaner.clean end - - # rspec-rails 3 will no longer automatically infer an example group's spec type - # from the file location. You can explicitly opt-in to the feature using this - # config option. - # To explicitly tag specs without using automatic inference, set the `:type` - # metadata manually: - # - # describe ThingsController, :type => :controller do - # # Equivalent to being in spec/controllers - # end - config.infer_spec_type_from_file_location! end diff --git a/spec/support/matchers.rb b/spec/support/matchers.rb index 5f0c270f967..e5ebc6e7ec8 100644 --- a/spec/support/matchers.rb +++ b/spec/support/matchers.rb @@ -1,30 +1,43 @@ RSpec::Matchers.define :be_valid_commit do match do |actual| - !actual.nil? - actual.id == ValidCommit::ID - actual.message == ValidCommit::MESSAGE - actual.author_name == ValidCommit::AUTHOR_FULL_NAME + actual && + actual.id == ValidCommit::ID && + actual.message == ValidCommit::MESSAGE && + actual.author_name == ValidCommit::AUTHOR_FULL_NAME end end +def emulate_user(user) + user = case user + when :user then create(:user) + when :visitor then nil + when :admin then create(:admin) + else user + end + login_with(user) if user +end + RSpec::Matchers.define :be_allowed_for do |user| match do |url| - include UrlAccess - url_allowed?(user, url) + emulate_user(user) + visit url + status_code != 404 && current_path != new_user_session_path end end RSpec::Matchers.define :be_denied_for do |user| match do |url| - include UrlAccess - url_denied?(user, url) + emulate_user(user) + visit url + status_code == 404 || current_path == new_user_session_path end end -RSpec::Matchers.define :be_404_for do |user| +RSpec::Matchers.define :be_not_found_for do |user| match do |url| - include UrlAccess - url_404?(user, url) + emulate_user(user) + visit url + status_code == 404 end end @@ -33,38 +46,12 @@ RSpec::Matchers.define :include_module do |expected| described_class.included_modules.include?(expected) end - failure_message_for_should do - "expected #{described_class} to include the #{expected} module" + description do + "includes the #{expected} module" end -end -module UrlAccess - def url_allowed?(user, url) - emulate_user(user) - visit url - (status_code != 404 && current_path != new_user_session_path) - end - - def url_denied?(user, url) - emulate_user(user) - visit url - (status_code == 404 || current_path == new_user_session_path) - end - - def url_404?(user, url) - emulate_user(user) - visit url - status_code == 404 - end - - def emulate_user(user) - user = case user - when :user then create(:user) - when :visitor then nil - when :admin then create(:admin) - else user - end - login_with(user) if user + failure_message do + "expected #{described_class} to include the #{expected} module" end end diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb index d29c8a55c82..a2a0b6905f9 100644 --- a/spec/support/mentionable_shared_examples.rb +++ b/spec/support/mentionable_shared_examples.rb @@ -80,7 +80,7 @@ shared_examples 'a mentionable' do ext_issue, ext_mr, ext_commit] mentioned_objects.each do |referenced| - expect(Note).to receive(:create_cross_reference_note). + expect(SystemNoteService).to receive(:cross_reference). with(referenced, subject.local_reference, author) end @@ -88,7 +88,7 @@ shared_examples 'a mentionable' do end it 'detects existing cross-references' do - Note.create_cross_reference_note(mentioned_issue, subject.local_reference, author) + SystemNoteService.cross_reference(mentioned_issue, subject.local_reference, author) expect(subject).to have_mentioned(mentioned_issue) expect(subject).not_to have_mentioned(mentioned_mr) @@ -132,13 +132,13 @@ shared_examples 'an editable mentionable' do # These three objects were already referenced, and should not receive new # notes [mentioned_issue, mentioned_commit, ext_issue].each do |oldref| - expect(Note).not_to receive(:create_cross_reference_note). + expect(SystemNoteService).not_to receive(:cross_reference). with(oldref, any_args) end # These two issues are new and should receive reference notes new_issues.each do |newref| - expect(Note).to receive(:create_cross_reference_note). + expect(SystemNoteService).to receive(:cross_reference). with(newref, subject.local_reference, author) end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 6d4a8067910..8bdd6b43cdd 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -41,11 +41,13 @@ module TestEnv end def disable_mailer - NotificationService.any_instance.stub(mailer: double.as_null_object) + allow_any_instance_of(NotificationService).to receive(:mailer). + and_return(double.as_null_object) end def enable_mailer - allow_any_instance_of(NotificationService).to receive(:mailer).and_call_original + allow_any_instance_of(NotificationService).to receive(:mailer). + and_call_original end # Clean /tmp/tests diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index c85214e50dd..2f90b67aef1 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -23,30 +23,33 @@ describe 'gitlab:app namespace rake task' do context 'gitlab version' do before do - Dir.stub glob: [] - allow(Dir).to receive :chdir - File.stub exists?: true - Kernel.stub system: true - FileUtils.stub cp_r: true - FileUtils.stub mv: true - Rake::Task["gitlab:shell:setup"].stub invoke: true + allow(Dir).to receive(:glob).and_return([]) + allow(Dir).to receive(:chdir) + allow(File).to receive(:exists?).and_return(true) + allow(Kernel).to receive(:system).and_return(true) + allow(FileUtils).to receive(:cp_r).and_return(true) + allow(FileUtils).to receive(:mv).and_return(true) + allow(Rake::Task["gitlab:shell:setup"]). + to receive(:invoke).and_return(true) end let(:gitlab_version) { Gitlab::VERSION } it 'should fail on mismatch' do - YAML.stub load_file: { gitlab_version: "not #{gitlab_version}" } - expect { run_rake_task('gitlab:backup:restore') }.to( - raise_error SystemExit - ) + allow(YAML).to receive(:load_file). + and_return({gitlab_version: "not #{gitlab_version}" }) + + expect { run_rake_task('gitlab:backup:restore') }. + to raise_error(SystemExit) end it 'should invoke restoration on mach' do - YAML.stub load_file: { gitlab_version: gitlab_version } - expect(Rake::Task["gitlab:backup:db:restore"]).to receive :invoke - expect(Rake::Task["gitlab:backup:repo:restore"]).to receive :invoke - expect(Rake::Task["gitlab:shell:setup"]).to receive :invoke - expect { run_rake_task('gitlab:backup:restore') }.to_not raise_error + allow(YAML).to receive(:load_file). + and_return({gitlab_version: gitlab_version}) + expect(Rake::Task["gitlab:backup:db:restore"]).to receive(:invoke) + expect(Rake::Task["gitlab:backup:repo:restore"]).to receive(:invoke) + expect(Rake::Task["gitlab:shell:setup"]).to receive(:invoke) + expect { run_rake_task('gitlab:backup:restore') }.not_to raise_error end end @@ -140,13 +143,14 @@ describe 'gitlab:app namespace rake task' do end it 'does not invoke repositories restore' do - Rake::Task["gitlab:shell:setup"].stub invoke: true + allow(Rake::Task["gitlab:shell:setup"]). + to receive(:invoke).and_return(true) allow($stdout).to receive :write expect(Rake::Task["gitlab:backup:db:restore"]).to receive :invoke expect(Rake::Task["gitlab:backup:repo:restore"]).not_to receive :invoke expect(Rake::Task["gitlab:shell:setup"]).to receive :invoke - expect { run_rake_task('gitlab:backup:restore') }.to_not raise_error + expect { run_rake_task('gitlab:backup:restore') }.not_to raise_error end end end # gitlab:app namespace diff --git a/spec/tasks/gitlab/mail_google_schema_whitelisting.rb b/spec/tasks/gitlab/mail_google_schema_whitelisting.rb index 22e746870dc..37feb5e6faf 100644 --- a/spec/tasks/gitlab/mail_google_schema_whitelisting.rb +++ b/spec/tasks/gitlab/mail_google_schema_whitelisting.rb @@ -21,7 +21,7 @@ describe 'gitlab:mail_google_schema_whitelisting rake task' do end it 'should run the task without errors' do - expect { run_rake_task }.to_not raise_error + expect { run_rake_task }.not_to raise_error end end end diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index df1a2b84a53..46eae9ab081 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -30,7 +30,7 @@ describe PostReceive do end it "asks the project to trigger all hooks" do - Project.stub(find_with_namespace: project) + allow(Project).to receive(:find_with_namespace).and_return(project) expect(project).to receive(:execute_hooks).twice expect(project).to receive(:execute_services).twice expect(project).to receive(:update_merge_requests) |