diff options
Diffstat (limited to 'spec')
101 files changed, 3122 insertions, 878 deletions
diff --git a/spec/benchmarks/lib/gitlab/markdown/reference_filter_spec.rb b/spec/benchmarks/lib/gitlab/markdown/reference_filter_spec.rb new file mode 100644 index 00000000000..34cd9f7e4eb --- /dev/null +++ b/spec/benchmarks/lib/gitlab/markdown/reference_filter_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +describe Gitlab::Markdown::ReferenceFilter, benchmark: true do + let(:input) do + html = <<-EOF +<p>Hello @alice and @bob, how are you doing today?</p> +<p>This is simple @dummy text to see how the @ReferenceFilter class performs +when @processing HTML.</p> + EOF + + Nokogiri::HTML.fragment(html) + end + + let(:project) { create(:empty_project) } + + let(:filter) { described_class.new(input, project: project) } + + describe '#replace_text_nodes_matching' do + let(:iterations) { 6000 } + + describe 'with identical input and output HTML' do + benchmark_subject do + filter.replace_text_nodes_matching(User.reference_pattern) do |content| + content + end + end + + it { is_expected.to iterate_per_second(iterations) } + end + + describe 'with different input and output HTML' do + benchmark_subject do + filter.replace_text_nodes_matching(User.reference_pattern) do |content| + '@eve' + end + end + + it { is_expected.to iterate_per_second(iterations) } + end + end +end diff --git a/spec/benchmarks/models/milestone_spec.rb b/spec/benchmarks/models/milestone_spec.rb new file mode 100644 index 00000000000..a94afc4c40d --- /dev/null +++ b/spec/benchmarks/models/milestone_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' + +describe Milestone, benchmark: true do + describe '#sort_issues' do + let(:milestone) { create(:milestone) } + + let(:issue1) { create(:issue, milestone: milestone) } + let(:issue2) { create(:issue, milestone: milestone) } + let(:issue3) { create(:issue, milestone: milestone) } + + let(:issue_ids) { [issue3.id, issue2.id, issue1.id] } + + benchmark_subject { milestone.sort_issues(issue_ids) } + + it { is_expected.to iterate_per_second(500) } + end +end diff --git a/spec/benchmarks/models/project_team_spec.rb b/spec/benchmarks/models/project_team_spec.rb new file mode 100644 index 00000000000..8b039ef7317 --- /dev/null +++ b/spec/benchmarks/models/project_team_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe ProjectTeam, benchmark: true do + describe '#max_member_access' do + let(:group) { create(:group) } + let(:project) { create(:empty_project, group: group) } + let(:user) { create(:user) } + + before do + project.team << [user, :master] + + 5.times do + project.team << [create(:user), :reporter] + + project.group.add_user(create(:user), :reporter) + end + end + + benchmark_subject { project.team.max_member_access(user.id) } + + it { is_expected.to iterate_per_second(35000) } + end +end diff --git a/spec/benchmarks/models/user_spec.rb b/spec/benchmarks/models/user_spec.rb index 168be20b7a5..1be7a8d3ed9 100644 --- a/spec/benchmarks/models/user_spec.rb +++ b/spec/benchmarks/models/user_spec.rb @@ -1,6 +1,16 @@ require 'spec_helper' describe User, benchmark: true do + describe '.all' do + before do + 10.times { create(:user) } + end + + benchmark_subject { User.all.to_a } + + it { is_expected.to iterate_per_second(500) } + end + describe '.by_login' do before do %w{Alice Bob Eve}.each do |name| @@ -11,7 +21,9 @@ describe User, benchmark: true do end end - let(:iterations) { 1000 } + # The iteration count is based on the query taking little over 1 ms when + # using PostgreSQL. + let(:iterations) { 900 } describe 'using a capitalized username' do benchmark_subject { User.by_login('Alice') } @@ -37,4 +49,30 @@ describe User, benchmark: true do it { is_expected.to iterate_per_second(iterations) } end end + + describe '.find_by_any_email' do + let(:user) { create(:user) } + + describe 'using a user with only a single Email address' do + let(:email) { user.email } + + benchmark_subject { User.find_by_any_email(email) } + + it { is_expected.to iterate_per_second(1000) } + end + + describe 'using a user with multiple Email addresses' do + let(:email) { user.emails.first.email } + + benchmark_subject { User.find_by_any_email(email) } + + before do + 10.times do + user.emails.create(email: FFaker::Internet.email) + end + end + + it { is_expected.to iterate_per_second(1000) } + end + end end diff --git a/spec/benchmarks/services/projects/create_service_spec.rb b/spec/benchmarks/services/projects/create_service_spec.rb new file mode 100644 index 00000000000..25ed48c34fd --- /dev/null +++ b/spec/benchmarks/services/projects/create_service_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe Projects::CreateService, benchmark: true do + describe '#execute' do + let(:user) { create(:user, :admin) } + + let(:group) do + group = create(:group) + + create(:group_member, group: group, user: user) + + group + end + + benchmark_subject do + name = SecureRandom.hex + service = described_class.new(user, + name: name, + path: name, + namespace_id: group.id, + visibility_level: Gitlab::VisibilityLevel::PUBLIC) + + service.execute + end + + it { is_expected.to iterate_per_second(0.5) } + end +end diff --git a/spec/controllers/abuse_reports_controller_spec.rb b/spec/controllers/abuse_reports_controller_spec.rb new file mode 100644 index 00000000000..0faab8d7ff0 --- /dev/null +++ b/spec/controllers/abuse_reports_controller_spec.rb @@ -0,0 +1,72 @@ +require 'spec_helper' + +describe AbuseReportsController do + let(:reporter) { create(:user) } + let(:user) { create(:user) } + let(:message) { "This user is a spammer" } + + before do + sign_in(reporter) + end + + describe "POST create" do + context "with admin notification email set" do + let(:admin_email) { "admin@example.com"} + + before(:each) do + stub_application_setting(admin_notification_email: admin_email) + end + + it "sends a notification email" do + post :create, + abuse_report: { + user_id: user.id, + message: message + } + + email = ActionMailer::Base.deliveries.last + + expect(email.to).to eq([admin_email]) + expect(email.subject).to include(user.username) + expect(email.text_part.body).to include(message) + end + + it "saves the abuse report" do + expect do + post :create, + abuse_report: { + user_id: user.id, + message: message + } + end.to change { AbuseReport.count }.by(1) + end + end + + context "without admin notification email set" do + before(:each) do + stub_application_setting(admin_notification_email: nil) + end + + it "does not send a notification email" do + expect do + post :create, + abuse_report: { + user_id: user.id, + message: message + } + end.not_to change { ActionMailer::Base.deliveries.count } + end + + it "saves the abuse report" do + expect do + post :create, + abuse_report: { + user_id: user.id, + message: message + } + end.to change { AbuseReport.count }.by(1) + end + end + end + +end diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 7168db117d6..fcbe62cace8 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -37,6 +37,32 @@ describe Admin::UsersController do end end + describe 'PUT block/:id' do + let(:user) { create(:user) } + + it 'blocks user' do + put :block, id: user.username + user.reload + expect(user.blocked?).to be_truthy + expect(flash[:notice]).to eq 'Successfully blocked' + end + end + + describe 'PUT unblock/:id' do + let(:user) { create(:user) } + + before do + user.block + end + + it 'unblocks user' do + put :unblock, id: user.username + user.reload + expect(user.blocked?).to be_falsey + expect(flash[:notice]).to eq 'Successfully unblocked' + end + end + describe 'PUT unlock/:id' do let(:user) { create(:user) } diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index 766be578f7f..bbf8adef534 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -41,7 +41,7 @@ describe Import::GithubController do it "assigns variables" do @project = create(:project, import_type: 'github', creator_id: user.id) - stub_client(repos: [@repo], orgs: [@org], org_repos: [@org_repo]) + stub_client(repos: [@repo, @org_repo], orgs: [@org], org_repos: [@org_repo]) get :status diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb new file mode 100644 index 00000000000..3c6e54839b5 --- /dev/null +++ b/spec/controllers/invites_controller_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe InvitesController do + let(:token) { '123456' } + let(:user) { create(:user) } + let(:member) { create(:project_member, invite_token: token, invite_email: 'test@abc.com', user: user) } + + before do + controller.instance_variable_set(:@member, member) + sign_in(user) + end + + describe 'GET #accept' do + it 'accepts user' do + get :accept, id: token + member.reload + + expect(response.status).to eq(302) + expect(member.user).to eq(user) + expect(flash[:notice]).to include 'You have been granted' + end + end + + describe 'GET #decline' do + it 'declines user' do + get :decline, id: token + expect{member.reload}.to raise_error ActiveRecord::RecordNotFound + + expect(response.status).to eq(302) + expect(flash[:notice]).to include 'You have declined the invitation to join' + end + end +end diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb index 91856ed0cc0..18a30033ed8 100644 --- a/spec/controllers/projects/repositories_controller_spec.rb +++ b/spec/controllers/projects/repositories_controller_spec.rb @@ -33,33 +33,5 @@ describe Projects::RepositoriesController do expect(response.status).to eq(404) end end - - context "when the service doesn't return a path" do - - before do - allow(service).to receive(:execute).and_return(nil) - end - - it "reloads the page" do - get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" - - expect(response).to redirect_to(archive_namespace_project_repository_path(project.namespace, project, ref: "master", format: "zip")) - end - end - - context "when the service returns a path" do - - let(:path) { Rails.root.join("spec/fixtures/dk.png").to_s } - - before do - allow(service).to receive(:execute).and_return(path) - end - - it "sends the file" do - get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" - - expect(response.body).to eq(File.binread(path)) - end - end end end diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb index d4ecd98e12d..ccd8c741c83 100644 --- a/spec/controllers/projects/services_controller_spec.rb +++ b/spec/controllers/projects/services_controller_spec.rb @@ -10,26 +10,43 @@ describe Projects::ServicesController do project.team << [user, :master] controller.instance_variable_set(:@project, project) controller.instance_variable_set(:@service, service) - request.env["HTTP_REFERER"] = "/" end - describe "#test" do - context 'success' do - it "should redirect and show success message" do - expect(service).to receive(:test).and_return({ success: true, result: 'done' }) - get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html - expect(response.status).to redirect_to('/') - expect(flash[:notice]).to eq('We sent a request to the provided URL') - end + shared_examples_for 'services controller' do |referrer| + before do + request.env["HTTP_REFERER"] = referrer end - context 'failure' do - it "should redirect and show failure message" do - expect(service).to receive(:test).and_return({ success: false, result: 'Bad test' }) - get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html - expect(response.status).to redirect_to('/') - expect(flash[:alert]).to eq('We tried to send a request to the provided URL but an error occurred: Bad test') + describe "#test" do + context 'success' do + it "should redirect and show success message" do + expect(service).to receive(:test).and_return({ success: true, result: 'done' }) + get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html + expect(response.status).to redirect_to('/') + expect(flash[:notice]).to eq('We sent a request to the provided URL') + end + end + + context 'failure' do + it "should redirect and show failure message" do + expect(service).to receive(:test).and_return({ success: false, result: 'Bad test' }) + get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html + expect(response.status).to redirect_to('/') + expect(flash[:alert]).to eq('We tried to send a request to the provided URL but an error occurred: Bad test') + end end end end + + describe 'referrer defined' do + it_should_behave_like 'services controller' do + let!(:referrer) { "/" } + end + end + + describe 'referrer undefined' do + it_should_behave_like 'services controller' do + let!(:referrer) { nil } + end + end end diff --git a/spec/controllers/projects/uploads_controller_spec.rb b/spec/controllers/projects/uploads_controller_spec.rb index f51abfedae5..93c4494c660 100644 --- a/spec/controllers/projects/uploads_controller_spec.rb +++ b/spec/controllers/projects/uploads_controller_spec.rb @@ -33,7 +33,7 @@ describe Projects::UploadsController do it 'returns a content with original filename, new link, and correct type.' do expect(response.body).to match '\"alt\":\"rails_sample\"' - expect(response.body).to match "\"url\":\"http://localhost/#{project.path_with_namespace}/uploads" + expect(response.body).to match "\"url\":\"/uploads" expect(response.body).to match '\"is_image\":true' end end @@ -49,7 +49,7 @@ describe Projects::UploadsController do it 'returns a content with original filename, new link, and correct type.' do expect(response.body).to match '\"alt\":\"doc_sample.txt\"' - expect(response.body).to match "\"url\":\"http://localhost/#{project.path_with_namespace}/uploads" + expect(response.body).to match "\"url\":\"/uploads" expect(response.body).to match '\"is_image\":false' end end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 21beaf37fce..4bb47c6b025 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -22,17 +22,68 @@ describe ProjectsController do end end - context "when requested with case sensitive namespace and project path" do - it "redirects to the normalized path for case mismatch" do - get :show, namespace_id: public_project.namespace.path, id: public_project.path.upcase + context "rendering default project view" do + render_views + + it "renders the activity view" do + allow(controller).to receive(:current_user).and_return(user) + allow(user).to receive(:project_view).and_return('activity') + + get :show, namespace_id: public_project.namespace.path, id: public_project.path + expect(response).to render_template('_activity') + end + + it "renders the readme view" do + allow(controller).to receive(:current_user).and_return(user) + allow(user).to receive(:project_view).and_return('readme') - expect(response).to redirect_to("/#{public_project.path_with_namespace}") + get :show, namespace_id: public_project.namespace.path, id: public_project.path + expect(response).to render_template('_readme') end - it "loads the page if normalized path matches request path" do + it "renders the files view" do + allow(controller).to receive(:current_user).and_return(user) + allow(user).to receive(:project_view).and_return('files') + get :show, namespace_id: public_project.namespace.path, id: public_project.path + expect(response).to render_template('_files') + end + end + + context "when requested with case sensitive namespace and project path" do + context "when there is a match with the same casing" do + it "loads the project" do + get :show, namespace_id: public_project.namespace.path, id: public_project.path + + expect(assigns(:project)).to eq(public_project) + expect(response.status).to eq(200) + end + end + + context "when there is a match with different casing" do + it "redirects to the normalized path" do + get :show, namespace_id: public_project.namespace.path, id: public_project.path.upcase + + expect(assigns(:project)).to eq(public_project) + expect(response).to redirect_to("/#{public_project.path_with_namespace}") + end + + + # MySQL queries are case insensitive by default, so this spec would fail. + if Gitlab::Database.postgresql? + context "when there is also a match with the same casing" do - expect(response.status).to eq(200) + let!(:other_project) { create(:project, :public, namespace: public_project.namespace, path: public_project.path.upcase) } + + it "loads the exactly matched project" do + + get :show, namespace_id: public_project.namespace.path, id: public_project.path.upcase + + expect(assigns(:project)).to eq(other_project) + expect(response.status).to eq(200) + end + end + end end end end @@ -62,4 +113,50 @@ describe ProjectsController do expect(user.starred?(public_project)).to be_falsey end end + + describe "DELETE remove_fork" do + context 'when signed in' do + before do + sign_in(user) + end + + context 'with forked project' do + let(:project_fork) { create(:project, namespace: user.namespace) } + + before do + create(:forked_project_link, forked_to_project: project_fork) + end + + it 'should remove fork from project' do + delete(:remove_fork, + namespace_id: project_fork.namespace.to_param, + id: project_fork.to_param, format: :js) + + expect(project_fork.forked?).to be_falsey + expect(flash[:notice]).to eq('The fork relationship has been removed.') + expect(response).to render_template(:remove_fork) + end + end + + context 'when project not forked' do + let(:unforked_project) { create(:project, namespace: user.namespace) } + + it 'should do nothing if project was not forked' do + delete(:remove_fork, + namespace_id: unforked_project.namespace.to_param, + id: unforked_project.to_param, format: :js) + + expect(flash[:notice]).to be_nil + expect(response).to render_template(:remove_fork) + end + end + end + + it "does nothing if user is not signed in" do + delete(:remove_fork, + namespace_id: project.namespace.to_param, + id: project.to_param, format: :js) + expect(response.status).to eq(401) + end + end end diff --git a/spec/factories/ci/projects.rb b/spec/factories/ci/projects.rb index 111e1a82816..1183a190353 100644 --- a/spec/factories/ci/projects.rb +++ b/spec/factories/ci/projects.rb @@ -33,6 +33,8 @@ FactoryGirl.define do gl_project factory: :empty_project + shared_runners_enabled false + factory :ci_project do token 'iPWx6WM4lhHNedGfBpPJNP' end diff --git a/spec/factories/releases.rb b/spec/factories/releases.rb new file mode 100644 index 00000000000..80d6bbee6c7 --- /dev/null +++ b/spec/factories/releases.rb @@ -0,0 +1,9 @@ +# Read about factories at https://github.com/thoughtbot/factory_girl + +FactoryGirl.define do + factory :release do + tag "v1.1.0" + description "Awesome release" + project + end +end diff --git a/spec/features/builds_spec.rb b/spec/features/builds_spec.rb index 924047a0d8f..158e85e598f 100644 --- a/spec/features/builds_spec.rb +++ b/spec/features/builds_spec.rb @@ -9,6 +9,55 @@ describe "Builds" do @gl_project.team << [@user, :master] end + describe "GET /:project/builds" do + context "Running scope" do + before do + @build.run! + visit namespace_project_builds_path(@gl_project.namespace, @gl_project) + end + + it { expect(page).to have_content 'Running' } + it { expect(page).to have_content 'Cancel all' } + it { expect(page).to have_content @build.short_sha } + it { expect(page).to have_content @build.ref } + it { expect(page).to have_content @build.name } + end + + context "Finished scope" do + before do + @build.run! + visit namespace_project_builds_path(@gl_project.namespace, @gl_project, scope: :finished) + end + + it { expect(page).to have_content 'No builds to show' } + it { expect(page).to have_content 'Cancel all' } + end + + context "All builds" do + before do + @gl_project.ci_builds.running_or_pending.each(&:success) + visit namespace_project_builds_path(@gl_project.namespace, @gl_project, scope: :all) + end + + it { expect(page).to have_content 'All' } + it { expect(page).to have_content @build.short_sha } + it { expect(page).to have_content @build.ref } + it { expect(page).to have_content @build.name } + it { expect(page).to_not have_content 'Cancel all' } + end + end + + describe "POST /:project/builds/:id/cancel_all" do + before do + @build.run! + visit namespace_project_builds_path(@gl_project.namespace, @gl_project) + click_link "Cancel all" + end + + it { expect(page).to have_content 'No builds to show' } + it { expect(page).to_not have_content 'Cancel all' } + end + describe "GET /:project/builds/:id" do before do visit namespace_project_build_path(@gl_project.namespace, @gl_project, @build) @@ -19,10 +68,11 @@ describe "Builds" do it { expect(page).to have_content @commit.git_author_name } end - describe "GET /:project/builds/:id/cancel" do + describe "POST /:project/builds/:id/cancel" do before do @build.run! - visit cancel_namespace_project_build_path(@gl_project.namespace, @gl_project, @build) + visit namespace_project_build_path(@gl_project.namespace, @gl_project, @build) + click_link "Cancel" end it { expect(page).to have_content 'canceled' } @@ -31,7 +81,9 @@ describe "Builds" do describe "POST /:project/builds/:id/retry" do before do - visit cancel_namespace_project_build_path(@gl_project.namespace, @gl_project, @build) + @build.run! + visit namespace_project_build_path(@gl_project.namespace, @gl_project, @build) + click_link "Cancel" click_link 'Retry' end diff --git a/spec/features/ci/events_spec.rb b/spec/features/ci/events_spec.rb deleted file mode 100644 index 5b9fd404159..00000000000 --- a/spec/features/ci/events_spec.rb +++ /dev/null @@ -1,22 +0,0 @@ -require 'spec_helper' - -describe "Events" do - let(:user) { create(:user) } - let(:project) { FactoryGirl.create :ci_project } - let(:event) { FactoryGirl.create :ci_admin_event, project: project } - - before do - login_as(user) - project.gl_project.team << [user, :master] - end - - describe "GET /ci/project/:id/events" do - before do - event - visit ci_project_events_path(project) - end - - it { expect(page).to have_content "Events" } - it { expect(page).to have_content event.description } - end -end diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index 1adc2cdf70a..340924fafe7 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -32,7 +32,7 @@ describe "Commits" do describe "Cancel all builds" do it "cancels commit" do visit ci_status_path(@commit) - click_on "Cancel all" + click_on "Cancel running" expect(page).to have_content "canceled" end end diff --git a/spec/features/markdown_spec.rb b/spec/features/markdown_spec.rb index c557a1061af..fdd8cf07b12 100644 --- a/spec/features/markdown_spec.rb +++ b/spec/features/markdown_spec.rb @@ -220,7 +220,7 @@ describe 'GitLab Markdown', feature: true do end end - # `markdown` calls these two methods + # Fake a `current_user` helper def current_user @feat.user end diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index aac93b17a38..09fcff2444a 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -34,6 +34,27 @@ feature 'Project', feature: true do end end + describe 'remove forked relationship', js: true do + let(:user) { create(:user) } + let(:project) { create(:project, namespace: user.namespace) } + + before do + login_with user + create(:forked_project_link, forked_to_project: project) + visit edit_namespace_project_path(project.namespace, project) + end + + it 'should remove fork' do + expect(page).to have_content 'Remove fork relationship' + + remove_with_confirm('Remove fork relationship', project.path) + + expect(page).to have_content 'The fork relationship has been removed.' + expect(project.forked?).to be_falsey + expect(page).not_to have_content 'Remove fork relationship' + end + end + describe 'removal', js: true do let(:user) { create(:user) } let(:project) { create(:project, namespace: user.namespace) } @@ -45,13 +66,13 @@ feature 'Project', feature: true do end it 'should remove project' do - expect { remove_project }.to change {Project.count}.by(-1) + expect { remove_with_confirm('Remove project', project.path) }.to change {Project.count}.by(-1) end end - def remove_project - click_button "Remove project" - fill_in 'confirm_name_input', with: project.path + def remove_with_confirm(button_text, confirm_with) + click_button button_text + fill_in 'confirm_name_input', with: confirm_with click_button 'Confirm' end end diff --git a/spec/finders/group_finder_spec.rb b/spec/finders/group_finder_spec.rb new file mode 100644 index 00000000000..78dc027837c --- /dev/null +++ b/spec/finders/group_finder_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe GroupsFinder do + let(:user) { create :user } + let!(:group) { create :group } + let!(:public_group) { create :group, public: true } + + describe :execute do + it 'finds public group' do + groups = GroupsFinder.new.execute(user) + expect(groups.size).to eq(1) + expect(groups.first).to eq(public_group) + end + end +end diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 742420f550e..1dfae0fbd3f 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -99,6 +99,15 @@ describe ApplicationHelper do helper.avatar_icon('foo@example.com', 20) end + + describe 'using a User' do + it 'should return an URL for the avatar' do + user = create(:user, avatar: File.open(avatar_file_path)) + + expect(helper.avatar_icon(user).to_s). + to match("/uploads/user/avatar/#{user.id}/banana_sample.gif") + end + end end describe 'gravatar_icon' do diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index 20ae29e2bd3..762ec25c4f5 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -11,12 +11,15 @@ describe GitlabMarkdownHelper do let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:snippet) { create(:project_snippet, project: project) } - # Helper expects a current_user method. - let(:current_user) { user } - before do + # Ensure the generated reference links aren't redacted + project.team << [user, :master] + # Helper expects a @project instance variable - @project = project + helper.instance_variable_set(:@project, project) + + # Stub the `current_user` helper + allow(helper).to receive(:current_user).and_return(user) end describe "#markdown" do @@ -25,23 +28,23 @@ describe GitlabMarkdownHelper do it "should link to the merge request" do expected = namespace_project_merge_request_path(project.namespace, project, merge_request) - expect(markdown(actual)).to match(expected) + expect(helper.markdown(actual)).to match(expected) end it "should link to the commit" do expected = namespace_project_commit_path(project.namespace, project, commit) - expect(markdown(actual)).to match(expected) + expect(helper.markdown(actual)).to match(expected) end it "should link to the issue" do expected = namespace_project_issue_path(project.namespace, project, issue) - expect(markdown(actual)).to match(expected) + expect(helper.markdown(actual)).to match(expected) end end describe "override default project" do let(:actual) { issue.to_reference } - let(:second_project) { create(:project) } + let(:second_project) { create(:project, :public) } let(:second_issue) { create(:issue, project: second_project) } it 'should link to the issue' do @@ -56,7 +59,7 @@ describe GitlabMarkdownHelper do let(:issues) { create_list(:issue, 2, project: project) } it 'should handle references nested in links with all the text' do - actual = link_to_gfm("This should finally fix #{issues[0].to_reference} and #{issues[1].to_reference} for real", commit_path) + actual = helper.link_to_gfm("This should finally fix #{issues[0].to_reference} and #{issues[1].to_reference} for real", commit_path) doc = Nokogiri::HTML.parse(actual) # Make sure we didn't create invalid markup @@ -86,7 +89,7 @@ describe GitlabMarkdownHelper do end it 'should forward HTML options' do - actual = link_to_gfm("Fixed in #{commit.id}", commit_path, class: 'foo') + actual = helper.link_to_gfm("Fixed in #{commit.id}", commit_path, class: 'foo') doc = Nokogiri::HTML.parse(actual) expect(doc.css('a')).to satisfy do |v| @@ -97,13 +100,13 @@ describe GitlabMarkdownHelper do it "escapes HTML passed in as the body" do actual = "This is a <h1>test</h1> - see #{issues[0].to_reference}" - expect(link_to_gfm(actual, commit_path)). + expect(helper.link_to_gfm(actual, commit_path)). to match('<h1>test</h1>') end it 'ignores reference links when they are the entire body' do text = issues[0].to_reference - act = link_to_gfm(text, '/foo') + act = helper.link_to_gfm(text, '/foo') expect(act).to eq %Q(<a href="/foo">#{issues[0].to_reference}</a>) end diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index c08ddb4cae1..78a6b631eb2 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -117,4 +117,14 @@ describe IssuesHelper do end end + describe "#merge_requests_sentence" do + subject { merge_requests_sentence(merge_requests)} + let(:merge_requests) do + [ build(:merge_request, iid: 1), build(:merge_request, iid: 2), + build(:merge_request, iid: 3)] + end + + it { is_expected.to eq("!1, !2, or !3") } + end + end diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb index fb70a36dc02..0c8d06b7059 100644 --- a/spec/helpers/labels_helper_spec.rb +++ b/spec/helpers/labels_helper_spec.rb @@ -14,11 +14,6 @@ describe LabelsHelper do expect(label).not_to receive(:project) link_to_label(label) end - - it 'includes option for "No Label"' do - result = project_labels_options(project) - expect(result).to include('No Label') - end end context 'without @project set' do diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 53e56ebff44..f2efb528aeb 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -70,4 +70,18 @@ describe ProjectsHelper do expect(helper.send(:readme_cache_key)).to eq("#{project.path_with_namespace}-nil-readme") end end + + describe 'link_to_member' do + let(:group) { create(:group) } + let(:project) { create(:empty_project, group: group) } + let(:user) { create(:user) } + + describe 'using the default options' do + it 'returns an HTML link to the user' do + link = helper.link_to_member(project, user) + + expect(link).to match(%r{/u/#{user.username}}) + end + end + end end diff --git a/spec/helpers/runners_helper_spec.rb b/spec/helpers/runners_helper_spec.rb index b3d635a1932..35f91b7decf 100644 --- a/spec/helpers/runners_helper_spec.rb +++ b/spec/helpers/runners_helper_spec.rb @@ -12,7 +12,7 @@ describe RunnersHelper do end it "returns online text" do - runner = FactoryGirl.build(:ci_runner, contacted_at: 1.hour.ago, active: true) + runner = FactoryGirl.build(:ci_runner, contacted_at: 1.second.ago, active: true) expect(runner_status_icon(runner)).to include("Runner is online") end end diff --git a/spec/helpers/search_helper_spec.rb b/spec/helpers/search_helper_spec.rb index b327f4f911a..ebe9c29d91c 100644 --- a/spec/helpers/search_helper_spec.rb +++ b/spec/helpers/search_helper_spec.rb @@ -42,6 +42,11 @@ describe SearchHelper do expect(search_autocomplete_opts(project.name).size).to eq(1) end + it "includes the public group" do + group = create(:group, public: true) + expect(search_autocomplete_opts(group.name).size).to eq(1) + end + context "with a current project" do before { @project = create(:project) } diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index aba957da488..9963f76f993 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' module Ci describe GitlabCiYamlProcessor do - + let(:path) { 'path' } + describe "#builds_for_ref" do let(:type) { 'test' } @@ -12,7 +13,7 @@ module Ci rspec: { script: "rspec" } }) - config_processor = GitlabCiYamlProcessor.new(config) + config_processor = GitlabCiYamlProcessor.new(config, path) expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(1) expect(config_processor.builds_for_stage_and_ref(type, "master").first).to eq({ @@ -24,81 +25,222 @@ module Ci commands: "pwd\nrspec", tag_list: [], options: {}, - allow_failure: false + allow_failure: false, + when: "on_success" }) end + + describe :only do + it "does not return builds if only has another branch" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", only: ["deploy"] } + }) - it "does not return builds if only has another branch" do - config = YAML.dump({ - before_script: ["pwd"], - rspec: { script: "rspec", only: ["deploy"] } - }) + config_processor = GitlabCiYamlProcessor.new(config, path) - config_processor = GitlabCiYamlProcessor.new(config) + expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(0) + end - expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(0) - end + it "does not return builds if only has regexp with another branch" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", only: ["/^deploy$/"] } + }) - it "does not return builds if only has regexp with another branch" do - config = YAML.dump({ - before_script: ["pwd"], - rspec: { script: "rspec", only: ["/^deploy$/"] } - }) + config_processor = GitlabCiYamlProcessor.new(config, path) - config_processor = GitlabCiYamlProcessor.new(config) + expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(0) + end - expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(0) - end + it "returns builds if only has specified this branch" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", only: ["master"] } + }) - it "returns builds if only has specified this branch" do - config = YAML.dump({ - before_script: ["pwd"], - rspec: { script: "rspec", only: ["master"] } - }) + config_processor = GitlabCiYamlProcessor.new(config, path) - config_processor = GitlabCiYamlProcessor.new(config) + expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(1) + end - expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(1) - end + it "returns builds if only has a list of branches including specified" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", type: type, only: ["master", "deploy"] } + }) - it "does not build tags" do - config = YAML.dump({ - before_script: ["pwd"], - rspec: { script: "rspec", except: ["tags"] } - }) + config_processor = GitlabCiYamlProcessor.new(config, path) - config_processor = GitlabCiYamlProcessor.new(config) + expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(1) + end - expect(config_processor.builds_for_stage_and_ref(type, "0-1", true).size).to eq(0) - end + it "returns builds if only has a branches keyword specified" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", type: type, only: ["branches"] } + }) - it "returns builds if only has a list of branches including specified" do - config = YAML.dump({ - before_script: ["pwd"], - rspec: { script: "rspec", type: type, only: ["master", "deploy"] } - }) + config_processor = GitlabCiYamlProcessor.new(config, path) + + expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(1) + end + + it "does not return builds if only has a tags keyword" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", type: type, only: ["tags"] } + }) + + config_processor = GitlabCiYamlProcessor.new(config, path) + + expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(0) + end + + it "returns builds if only has current repository path" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", type: type, only: ["branches@path"] } + }) + + config_processor = GitlabCiYamlProcessor.new(config, path) - config_processor = GitlabCiYamlProcessor.new(config) + expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(1) + end - expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(1) + it "does not return builds if only has different repository path" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", type: type, only: ["branches@fork"] } + }) + + config_processor = GitlabCiYamlProcessor.new(config, path) + + expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(0) + end + + it "returns build only for specified type" do + + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", type: "test", only: ["master", "deploy"] }, + staging: { script: "deploy", type: "deploy", only: ["master", "deploy"] }, + production: { script: "deploy", type: "deploy", only: ["master@path", "deploy"] }, + }) + + config_processor = GitlabCiYamlProcessor.new(config, 'fork') + + expect(config_processor.builds_for_stage_and_ref("deploy", "deploy").size).to eq(2) + expect(config_processor.builds_for_stage_and_ref("test", "deploy").size).to eq(1) + expect(config_processor.builds_for_stage_and_ref("deploy", "master").size).to eq(1) + end end - it "returns build only for specified type" do + describe :except do + it "returns builds if except has another branch" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", except: ["deploy"] } + }) - config = YAML.dump({ - before_script: ["pwd"], - build: { script: "build", type: "build", only: ["master", "deploy"] }, - rspec: { script: "rspec", type: type, only: ["master", "deploy"] }, - staging: { script: "deploy", type: "deploy", only: ["master", "deploy"] }, - production: { script: "deploy", type: "deploy", only: ["master", "deploy"] }, - }) + config_processor = GitlabCiYamlProcessor.new(config, path) - config_processor = GitlabCiYamlProcessor.new(config) + expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(1) + end - expect(config_processor.builds_for_stage_and_ref("production", "deploy").size).to eq(0) - expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(1) - expect(config_processor.builds_for_stage_and_ref("deploy", "deploy").size).to eq(2) + it "returns builds if except has regexp with another branch" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", except: ["/^deploy$/"] } + }) + + config_processor = GitlabCiYamlProcessor.new(config, path) + + expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(1) + end + + it "does not return builds if except has specified this branch" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", except: ["master"] } + }) + + config_processor = GitlabCiYamlProcessor.new(config, path) + + expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(0) + end + + it "does not return builds if except has a list of branches including specified" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", type: type, except: ["master", "deploy"] } + }) + + config_processor = GitlabCiYamlProcessor.new(config, path) + + expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(0) + end + + it "does not return builds if except has a branches keyword specified" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", type: type, except: ["branches"] } + }) + + config_processor = GitlabCiYamlProcessor.new(config, path) + + expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(0) + end + + it "returns builds if except has a tags keyword" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", type: type, except: ["tags"] } + }) + + config_processor = GitlabCiYamlProcessor.new(config, path) + + expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(1) + end + + it "does not return builds if except has current repository path" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", type: type, except: ["branches@path"] } + }) + + config_processor = GitlabCiYamlProcessor.new(config, path) + + expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(0) + end + + it "returns builds if except has different repository path" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", type: type, except: ["branches@fork"] } + }) + + config_processor = GitlabCiYamlProcessor.new(config, path) + + expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(1) + end + + it "returns build except specified type" do + config = YAML.dump({ + before_script: ["pwd"], + rspec: { script: "rspec", type: "test", except: ["master", "deploy", "test@fork"] }, + staging: { script: "deploy", type: "deploy", except: ["master"] }, + production: { script: "deploy", type: "deploy", except: ["master@fork"] }, + }) + + config_processor = GitlabCiYamlProcessor.new(config, 'fork') + + expect(config_processor.builds_for_stage_and_ref("deploy", "deploy").size).to eq(2) + expect(config_processor.builds_for_stage_and_ref("test", "test").size).to eq(0) + expect(config_processor.builds_for_stage_and_ref("deploy", "master").size).to eq(0) + end end + end describe "Image and service handling" do @@ -110,7 +252,7 @@ module Ci rspec: { script: "rspec" } }) - config_processor = GitlabCiYamlProcessor.new(config) + config_processor = GitlabCiYamlProcessor.new(config, path) expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1) expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({ @@ -125,7 +267,8 @@ module Ci image: "ruby:2.1", services: ["mysql"] }, - allow_failure: false + allow_failure: false, + when: "on_success" }) end @@ -137,7 +280,7 @@ module Ci rspec: { image: "ruby:2.5", services: ["postgresql"], script: "rspec" } }) - config_processor = GitlabCiYamlProcessor.new(config) + config_processor = GitlabCiYamlProcessor.new(config, path) expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1) expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({ @@ -152,7 +295,8 @@ module Ci image: "ruby:2.5", services: ["postgresql"] }, - allow_failure: false + allow_failure: false, + when: "on_success" }) end end @@ -169,11 +313,26 @@ module Ci rspec: { script: "rspec" } }) - config_processor = GitlabCiYamlProcessor.new(config) + config_processor = GitlabCiYamlProcessor.new(config, path) expect(config_processor.variables).to eq(variables) end end + describe "When" do + %w(on_success on_failure always).each do |when_state| + it "returns #{when_state} when defined" do + config = YAML.dump({ + rspec: { script: "rspec", when: when_state } + }) + + config_processor = GitlabCiYamlProcessor.new(config, path) + builds = config_processor.builds_for_stage_and_ref("test", "master") + expect(builds.size).to eq(1) + expect(builds.first[:when]).to eq(when_state) + end + end + end + describe "Error handling" do it "indicates that object is invalid" do expect{GitlabCiYamlProcessor.new("invalid_yaml\n!ccdvlf%612334@@@@")}.to raise_error(GitlabCiYamlProcessor::ValidationError) @@ -182,135 +341,156 @@ module Ci it "returns errors if tags parameter is invalid" do config = YAML.dump({ rspec: { script: "test", tags: "mysql" } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: tags parameter should be an array of strings") end it "returns errors if before_script parameter is invalid" do config = YAML.dump({ before_script: "bundle update", rspec: { script: "test" } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "before_script should be an array of strings") end it "returns errors if image parameter is invalid" do config = YAML.dump({ image: ["test"], rspec: { script: "test" } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "image should be a string") end + it "returns errors if job name is blank" do + config = YAML.dump({ '' => { script: "test" } }) + expect do + GitlabCiYamlProcessor.new(config, path) + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "job name should be non-empty string") + end + + it "returns errors if job name is non-string" do + config = YAML.dump({ 10 => { script: "test" } }) + expect do + GitlabCiYamlProcessor.new(config, path) + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "job name should be non-empty string") + end + it "returns errors if job image parameter is invalid" do config = YAML.dump({ rspec: { script: "test", image: ["test"] } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: image should be a string") end it "returns errors if services parameter is not an array" do config = YAML.dump({ services: "test", rspec: { script: "test" } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services should be an array of strings") end it "returns errors if services parameter is not an array of strings" do config = YAML.dump({ services: [10, "test"], rspec: { script: "test" } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services should be an array of strings") end it "returns errors if job services parameter is not an array" do config = YAML.dump({ rspec: { script: "test", services: "test" } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: services should be an array of strings") end it "returns errors if job services parameter is not an array of strings" do config = YAML.dump({ rspec: { script: "test", services: [10, "test"] } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: services should be an array of strings") end it "returns errors if there are unknown parameters" do config = YAML.dump({ extra: "bundle update" }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Unknown parameter: extra") end it "returns errors if there are unknown parameters that are hashes, but doesn't have a script" do config = YAML.dump({ extra: { services: "test" } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Unknown parameter: extra") end it "returns errors if there is no any jobs defined" do config = YAML.dump({ before_script: ["bundle update"] }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Please define at least one job") end it "returns errors if job allow_failure parameter is not an boolean" do config = YAML.dump({ rspec: { script: "test", allow_failure: "string" } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: allow_failure parameter should be an boolean") end it "returns errors if job stage is not a string" do config = YAML.dump({ rspec: { script: "test", type: 1, allow_failure: "string" } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test, deploy") end it "returns errors if job stage is not a pre-defined stage" do config = YAML.dump({ rspec: { script: "test", type: "acceptance", allow_failure: "string" } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test, deploy") end it "returns errors if job stage is not a defined stage" do config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", type: "acceptance", allow_failure: "string" } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test") end it "returns errors if stages is not an array" do config = YAML.dump({ types: "test", rspec: { script: "test" } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages should be an array of strings") end it "returns errors if stages is not an array of strings" do config = YAML.dump({ types: [true, "test"], rspec: { script: "test" } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages should be an array of strings") end it "returns errors if variables is not a map" do config = YAML.dump({ variables: "test", rspec: { script: "test" } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables should be a map of key-valued strings") end it "returns errors if variables is not a map of key-valued strings" do config = YAML.dump({ variables: { test: false }, rspec: { script: "test" } }) expect do - GitlabCiYamlProcessor.new(config) + GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables should be a map of key-valued strings") end + + it "returns errors if job when is not on_success, on_failure or always" do + config = YAML.dump({ rspec: { script: "test", when: 1 } }) + expect do + GitlabCiYamlProcessor.new(config, path) + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: when parameter should be on_success, on_failure or always") + end end end end diff --git a/spec/lib/gitlab/backend/grack_auth_spec.rb b/spec/lib/gitlab/backend/grack_auth_spec.rb index 37c527221a0..dfa0e10318a 100644 --- a/spec/lib/gitlab/backend/grack_auth_spec.rb +++ b/spec/lib/gitlab/backend/grack_auth_spec.rb @@ -50,6 +50,22 @@ describe Grack::Auth do end end + context "when the Wiki for a project exists" do + before do + @wiki = ProjectWiki.new(project) + env["PATH_INFO"] = "#{@wiki.repository.path_with_namespace}.git/info/refs" + project.update_attribute(:visibility_level, Project::PUBLIC) + end + + it "responds with the right project" do + response = auth.call(env) + json_body = ActiveSupport::JSON.decode(response[2][0]) + + expect(response.first).to eq(200) + expect(json_body['RepoPath']).to include(@wiki.repository.path_with_namespace) + end + end + context "when the project exists" do before do env["PATH_INFO"] = project.path_with_namespace + ".git" diff --git a/spec/lib/gitlab/closing_issue_extractor_spec.rb b/spec/lib/gitlab/closing_issue_extractor_spec.rb index 5d7ff4f6122..21254f778d3 100644 --- a/spec/lib/gitlab/closing_issue_extractor_spec.rb +++ b/spec/lib/gitlab/closing_issue_extractor_spec.rb @@ -140,28 +140,28 @@ describe Gitlab::ClosingIssueExtractor do message = "Closes #{reference} and fix #{reference2}" expect(subject.closed_by_message(message)). - to eq([issue, other_issue]) + to match_array([issue, other_issue]) end it 'fetches comma-separated issues references in single line message' do message = "Closes #{reference}, closes #{reference2}" expect(subject.closed_by_message(message)). - to eq([issue, other_issue]) + to match_array([issue, other_issue]) end it 'fetches comma-separated issues numbers in single line message' do message = "Closes #{reference}, #{reference2} and #{reference3}" expect(subject.closed_by_message(message)). - to eq([issue, other_issue, third_issue]) + to match_array([issue, other_issue, third_issue]) end it 'fetches issues in multi-line message' do message = "Awesome commit (closes #{reference})\nAlso fixes #{reference2}" expect(subject.closed_by_message(message)). - to eq([issue, other_issue]) + to match_array([issue, other_issue]) end it 'fetches issues in hybrid message' do @@ -169,7 +169,7 @@ describe Gitlab::ClosingIssueExtractor do "Also fixing issues #{reference2}, #{reference3} and #4" expect(subject.closed_by_message(message)). - to eq([issue, other_issue, third_issue]) + to match_array([issue, other_issue, third_issue]) end end end diff --git a/spec/lib/gitlab/email/attachment_uploader_spec.rb b/spec/lib/gitlab/email/attachment_uploader_spec.rb index e8208e15e29..8fb432367b6 100644 --- a/spec/lib/gitlab/email/attachment_uploader_spec.rb +++ b/spec/lib/gitlab/email/attachment_uploader_spec.rb @@ -13,7 +13,6 @@ describe Gitlab::Email::AttachmentUploader do expect(link).not_to be_nil expect(link[:is_image]).to be_truthy expect(link[:alt]).to eq("bricks") - expect(link[:url]).to include("/#{project.path_with_namespace}") expect(link[:url]).to include("bricks.png") end end diff --git a/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb b/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb index 3c6c84a0416..e5b8d723fe5 100644 --- a/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb @@ -4,7 +4,7 @@ module Gitlab::Markdown describe CommitRangeReferenceFilter do include FilterSpecHelper - let(:project) { create(:project) } + let(:project) { create(:project, :public) } let(:commit1) { project.commit } let(:commit2) { project.commit("HEAD~2") } @@ -75,12 +75,20 @@ module Gitlab::Markdown expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-commit_range' end - it 'includes a data-project-id attribute' do + it 'includes a data-project attribute' do doc = filter("See #{reference}") link = doc.css('a').first - expect(link).to have_attribute('data-project-id') - expect(link.attr('data-project-id')).to eq project.id.to_s + expect(link).to have_attribute('data-project') + expect(link.attr('data-project')).to eq project.id.to_s + end + + it 'includes a data-commit-range attribute' do + doc = filter("See #{reference}") + link = doc.css('a').first + + expect(link).to have_attribute('data-commit-range') + expect(link.attr('data-commit-range')).to eq range.to_reference end it 'supports an :only_path option' do @@ -92,59 +100,45 @@ module Gitlab::Markdown end it 'adds to the results hash' do - result = pipeline_result("See #{reference}") + result = reference_pipeline_result("See #{reference}") expect(result[:references][:commit_range]).not_to be_empty end end context 'cross-project reference' do let(:namespace) { create(:namespace, name: 'cross-reference') } - let(:project2) { create(:project, namespace: namespace) } + let(:project2) { create(:project, :public, namespace: namespace) } let(:reference) { range.to_reference(project) } before do range.project = project2 end - context 'when user can access reference' do - before { allow_cross_reference! } - - it 'links to a valid reference' do - doc = filter("See #{reference}") - - expect(doc.css('a').first.attr('href')). - to eq urls.namespace_project_compare_url(project2.namespace, project2, range.to_param) - end - - it 'links with adjacent text' do - doc = filter("Fixed (#{reference}.)") - - exp = Regexp.escape("#{project2.to_reference}@#{range.to_s}") - expect(doc.to_html).to match(/\(<a.+>#{exp}<\/a>\.\)/) - end + it 'links to a valid reference' do + doc = filter("See #{reference}") - it 'ignores invalid commit IDs on the referenced project' do - exp = act = "Fixed #{project2.to_reference}@#{commit1.id.reverse}...#{commit2.id}" - expect(filter(act).to_html).to eq exp + expect(doc.css('a').first.attr('href')). + to eq urls.namespace_project_compare_url(project2.namespace, project2, range.to_param) + end - exp = act = "Fixed #{project2.to_reference}@#{commit1.id}...#{commit2.id.reverse}" - expect(filter(act).to_html).to eq exp - end + it 'links with adjacent text' do + doc = filter("Fixed (#{reference}.)") - it 'adds to the results hash' do - result = pipeline_result("See #{reference}") - expect(result[:references][:commit_range]).not_to be_empty - end + exp = Regexp.escape("#{project2.to_reference}@#{range.to_s}") + expect(doc.to_html).to match(/\(<a.+>#{exp}<\/a>\.\)/) end - context 'when user cannot access reference' do - before { disallow_cross_reference! } + it 'ignores invalid commit IDs on the referenced project' do + exp = act = "Fixed #{project2.to_reference}@#{commit1.id.reverse}...#{commit2.id}" + expect(filter(act).to_html).to eq exp - it 'ignores valid references' do - exp = act = "See #{reference}" + exp = act = "Fixed #{project2.to_reference}@#{commit1.id}...#{commit2.id.reverse}" + expect(filter(act).to_html).to eq exp + end - expect(filter(act).to_html).to eq exp - end + it 'adds to the results hash' do + result = reference_pipeline_result("See #{reference}") + expect(result[:references][:commit_range]).not_to be_empty end end end diff --git a/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb b/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb index 9ed438252b3..d080efbf3d4 100644 --- a/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb @@ -4,7 +4,7 @@ module Gitlab::Markdown describe CommitReferenceFilter do include FilterSpecHelper - let(:project) { create(:project) } + let(:project) { create(:project, :public) } let(:commit) { project.commit } it 'requires project context' do @@ -71,12 +71,20 @@ module Gitlab::Markdown expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-commit' end - it 'includes a data-project-id attribute' do + it 'includes a data-project attribute' do doc = filter("See #{reference}") link = doc.css('a').first - expect(link).to have_attribute('data-project-id') - expect(link.attr('data-project-id')).to eq project.id.to_s + expect(link).to have_attribute('data-project') + expect(link.attr('data-project')).to eq project.id.to_s + end + + it 'includes a data-commit attribute' do + doc = filter("See #{reference}") + link = doc.css('a').first + + expect(link).to have_attribute('data-commit') + expect(link.attr('data-commit')).to eq commit.id end it 'supports an :only_path context' do @@ -88,53 +96,39 @@ module Gitlab::Markdown end it 'adds to the results hash' do - result = pipeline_result("See #{reference}") + result = reference_pipeline_result("See #{reference}") expect(result[:references][:commit]).not_to be_empty end end context 'cross-project reference' do let(:namespace) { create(:namespace, name: 'cross-reference') } - let(:project2) { create(:project, namespace: namespace) } + let(:project2) { create(:project, :public, namespace: namespace) } let(:commit) { project2.commit } let(:reference) { commit.to_reference(project) } - context 'when user can access reference' do - before { allow_cross_reference! } - - it 'links to a valid reference' do - doc = filter("See #{reference}") - - expect(doc.css('a').first.attr('href')). - to eq urls.namespace_project_commit_url(project2.namespace, project2, commit.id) - end - - it 'links with adjacent text' do - doc = filter("Fixed (#{reference}.)") + it 'links to a valid reference' do + doc = filter("See #{reference}") - exp = Regexp.escape(project2.to_reference) - expect(doc.to_html).to match(/\(<a.+>#{exp}@#{commit.short_id}<\/a>\.\)/) - end + expect(doc.css('a').first.attr('href')). + to eq urls.namespace_project_commit_url(project2.namespace, project2, commit.id) + end - it 'ignores invalid commit IDs on the referenced project' do - exp = act = "Committed #{invalidate_reference(reference)}" - expect(filter(act).to_html).to eq exp - end + it 'links with adjacent text' do + doc = filter("Fixed (#{reference}.)") - it 'adds to the results hash' do - result = pipeline_result("See #{reference}") - expect(result[:references][:commit]).not_to be_empty - end + exp = Regexp.escape(project2.to_reference) + expect(doc.to_html).to match(/\(<a.+>#{exp}@#{commit.short_id}<\/a>\.\)/) end - context 'when user cannot access reference' do - before { disallow_cross_reference! } - - it 'ignores valid references' do - exp = act = "See #{reference}" + it 'ignores invalid commit IDs on the referenced project' do + exp = act = "Committed #{invalidate_reference(reference)}" + expect(filter(act).to_html).to eq exp + end - expect(filter(act).to_html).to eq exp - end + it 'adds to the results hash' do + result = reference_pipeline_result("See #{reference}") + expect(result[:references][:commit]).not_to be_empty end end end diff --git a/spec/lib/gitlab/markdown/cross_project_reference_spec.rb b/spec/lib/gitlab/markdown/cross_project_reference_spec.rb index 4698d6138c2..8d4f9e403a6 100644 --- a/spec/lib/gitlab/markdown/cross_project_reference_spec.rb +++ b/spec/lib/gitlab/markdown/cross_project_reference_spec.rb @@ -2,20 +2,16 @@ require 'spec_helper' module Gitlab::Markdown describe CrossProjectReference do - # context in the html-pipeline sense, not in the rspec sense - let(:context) do - { - current_user: double('user'), - project: double('project') - } - end - include described_class describe '#project_from_ref' do context 'when no project was referenced' do it 'returns the project from context' do - expect(project_from_ref(nil)).to eq context[:project] + project = double + + allow(self).to receive(:context).and_return({ project: project }) + + expect(project_from_ref(nil)).to eq project end end @@ -26,29 +22,13 @@ module Gitlab::Markdown end context 'when referenced project exists' do - let(:project2) { double('referenced project') } + it 'returns the referenced project' do + project2 = double('referenced project') - before do expect(Project).to receive(:find_with_namespace). with('cross/reference').and_return(project2) - end - - context 'and the user has permission to read it' do - it 'returns the referenced project' do - expect(self).to receive(:user_can_reference_project?). - with(project2).and_return(true) - - expect(project_from_ref('cross/reference')).to eq project2 - end - end - - context 'and the user does not have permission to read it' do - it 'returns nil' do - expect(self).to receive(:user_can_reference_project?). - with(project2).and_return(false) - expect(project_from_ref('cross/reference')).to be_nil - end + expect(project_from_ref('cross/reference')).to eq project2 end end end diff --git a/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb b/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb index 1dd54f58588..94c80ae6611 100644 --- a/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb @@ -8,7 +8,7 @@ module Gitlab::Markdown IssuesHelper end - let(:project) { create(:empty_project) } + let(:project) { create(:empty_project, :public) } let(:issue) { create(:issue, project: project) } it 'requires project context' do @@ -68,12 +68,20 @@ module Gitlab::Markdown expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-issue' end - it 'includes a data-project-id attribute' do + it 'includes a data-project attribute' do doc = filter("Issue #{reference}") link = doc.css('a').first - expect(link).to have_attribute('data-project-id') - expect(link.attr('data-project-id')).to eq project.id.to_s + expect(link).to have_attribute('data-project') + expect(link.attr('data-project')).to eq project.id.to_s + end + + it 'includes a data-issue attribute' do + doc = filter("See #{reference}") + link = doc.css('a').first + + expect(link).to have_attribute('data-issue') + expect(link.attr('data-issue')).to eq issue.id.to_s end it 'supports an :only_path context' do @@ -85,60 +93,46 @@ module Gitlab::Markdown end it 'adds to the results hash' do - result = pipeline_result("Fixed #{reference}") + result = reference_pipeline_result("Fixed #{reference}") expect(result[:references][:issue]).to eq [issue] end end context 'cross-project reference' do let(:namespace) { create(:namespace, name: 'cross-reference') } - let(:project2) { create(:empty_project, namespace: namespace) } + let(:project2) { create(:empty_project, :public, namespace: namespace) } let(:issue) { create(:issue, project: project2) } let(:reference) { issue.to_reference(project) } - context 'when user can access reference' do - before { allow_cross_reference! } - - it 'ignores valid references when cross-reference project uses external tracker' do - expect_any_instance_of(Project).to receive(:get_issue). - with(issue.iid).and_return(nil) - - exp = act = "Issue #{reference}" - expect(filter(act).to_html).to eq exp - end + it 'ignores valid references when cross-reference project uses external tracker' do + expect_any_instance_of(Project).to receive(:get_issue). + with(issue.iid).and_return(nil) - it 'links to a valid reference' do - doc = filter("See #{reference}") - - expect(doc.css('a').first.attr('href')). - to eq helper.url_for_issue(issue.iid, project2) - end - - it 'links with adjacent text' do - doc = filter("Fixed (#{reference}.)") - expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/) - end + exp = act = "Issue #{reference}" + expect(filter(act).to_html).to eq exp + end - it 'ignores invalid issue IDs on the referenced project' do - exp = act = "Fixed #{invalidate_reference(reference)}" + it 'links to a valid reference' do + doc = filter("See #{reference}") - expect(filter(act).to_html).to eq exp - end + expect(doc.css('a').first.attr('href')). + to eq helper.url_for_issue(issue.iid, project2) + end - it 'adds to the results hash' do - result = pipeline_result("Fixed #{reference}") - expect(result[:references][:issue]).to eq [issue] - end + it 'links with adjacent text' do + doc = filter("Fixed (#{reference}.)") + expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/) end - context 'when user cannot access reference' do - before { disallow_cross_reference! } + it 'ignores invalid issue IDs on the referenced project' do + exp = act = "Fixed #{invalidate_reference(reference)}" - it 'ignores valid references' do - exp = act = "See #{reference}" + expect(filter(act).to_html).to eq exp + end - expect(filter(act).to_html).to eq exp - end + it 'adds to the results hash' do + result = reference_pipeline_result("Fixed #{reference}") + expect(result[:references][:issue]).to eq [issue] end end end diff --git a/spec/lib/gitlab/markdown/label_reference_filter_spec.rb b/spec/lib/gitlab/markdown/label_reference_filter_spec.rb index e32089de376..fc21b65a843 100644 --- a/spec/lib/gitlab/markdown/label_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/label_reference_filter_spec.rb @@ -5,7 +5,7 @@ module Gitlab::Markdown describe LabelReferenceFilter do include FilterSpecHelper - let(:project) { create(:empty_project) } + let(:project) { create(:empty_project, :public) } let(:label) { create(:label, project: project) } let(:reference) { label.to_reference } @@ -25,12 +25,20 @@ module Gitlab::Markdown expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-label' end - it 'includes a data-project-id attribute' do + it 'includes a data-project attribute' do doc = filter("Label #{reference}") link = doc.css('a').first - expect(link).to have_attribute('data-project-id') - expect(link.attr('data-project-id')).to eq project.id.to_s + expect(link).to have_attribute('data-project') + expect(link.attr('data-project')).to eq project.id.to_s + end + + it 'includes a data-label attribute' do + doc = filter("See #{reference}") + link = doc.css('a').first + + expect(link).to have_attribute('data-label') + expect(link.attr('data-label')).to eq label.id.to_s end it 'supports an :only_path context' do @@ -42,7 +50,7 @@ module Gitlab::Markdown end it 'adds to the results hash' do - result = pipeline_result("Label #{reference}") + result = reference_pipeline_result("Label #{reference}") expect(result[:references][:label]).to eq [label] end diff --git a/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb b/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb index 66616b93368..3ef6cdfff33 100644 --- a/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb @@ -4,7 +4,7 @@ module Gitlab::Markdown describe MergeRequestReferenceFilter do include FilterSpecHelper - let(:project) { create(:project) } + let(:project) { create(:project, :public) } let(:merge) { create(:merge_request, source_project: project) } it 'requires project context' do @@ -56,12 +56,20 @@ module Gitlab::Markdown expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-merge_request' end - it 'includes a data-project-id attribute' do + it 'includes a data-project attribute' do doc = filter("Merge #{reference}") link = doc.css('a').first - expect(link).to have_attribute('data-project-id') - expect(link.attr('data-project-id')).to eq project.id.to_s + expect(link).to have_attribute('data-project') + expect(link.attr('data-project')).to eq project.id.to_s + end + + it 'includes a data-merge-request attribute' do + doc = filter("See #{reference}") + link = doc.css('a').first + + expect(link).to have_attribute('data-merge-request') + expect(link.attr('data-merge-request')).to eq merge.id.to_s end it 'supports an :only_path context' do @@ -73,53 +81,39 @@ module Gitlab::Markdown end it 'adds to the results hash' do - result = pipeline_result("Merge #{reference}") + result = reference_pipeline_result("Merge #{reference}") expect(result[:references][:merge_request]).to eq [merge] end end context 'cross-project reference' do let(:namespace) { create(:namespace, name: 'cross-reference') } - let(:project2) { create(:project, namespace: namespace) } + let(:project2) { create(:project, :public, namespace: namespace) } let(:merge) { create(:merge_request, source_project: project2) } let(:reference) { merge.to_reference(project) } - context 'when user can access reference' do - before { allow_cross_reference! } - - it 'links to a valid reference' do - doc = filter("See #{reference}") - - expect(doc.css('a').first.attr('href')). - to eq urls.namespace_project_merge_request_url(project2.namespace, - project, merge) - end - - it 'links with adjacent text' do - doc = filter("Merge (#{reference}.)") - expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/) - end - - it 'ignores invalid merge IDs on the referenced project' do - exp = act = "Merge #{invalidate_reference(reference)}" + it 'links to a valid reference' do + doc = filter("See #{reference}") - expect(filter(act).to_html).to eq exp - end + expect(doc.css('a').first.attr('href')). + to eq urls.namespace_project_merge_request_url(project2.namespace, + project, merge) + end - it 'adds to the results hash' do - result = pipeline_result("Merge #{reference}") - expect(result[:references][:merge_request]).to eq [merge] - end + it 'links with adjacent text' do + doc = filter("Merge (#{reference}.)") + expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/) end - context 'when user cannot access reference' do - before { disallow_cross_reference! } + it 'ignores invalid merge IDs on the referenced project' do + exp = act = "Merge #{invalidate_reference(reference)}" - it 'ignores valid references' do - exp = act = "See #{reference}" + expect(filter(act).to_html).to eq exp + end - expect(filter(act).to_html).to eq exp - end + it 'adds to the results hash' do + result = reference_pipeline_result("Merge #{reference}") + expect(result[:references][:merge_request]).to eq [merge] end end end diff --git a/spec/lib/gitlab/markdown/redactor_filter_spec.rb b/spec/lib/gitlab/markdown/redactor_filter_spec.rb new file mode 100644 index 00000000000..eea3f1cf370 --- /dev/null +++ b/spec/lib/gitlab/markdown/redactor_filter_spec.rb @@ -0,0 +1,91 @@ +require 'spec_helper' + +module Gitlab::Markdown + describe RedactorFilter do + include ActionView::Helpers::UrlHelper + include FilterSpecHelper + + it 'ignores non-GFM links' do + html = %(See <a href="https://google.com/">Google</a>) + doc = filter(html, current_user: double) + + expect(doc.css('a').length).to eq 1 + end + + def reference_link(data) + link_to('text', '', class: 'gfm', data: data) + end + + context 'with data-project' do + it 'removes unpermitted Project references' do + user = create(:user) + project = create(:empty_project) + + link = reference_link(project: project.id, reference_filter: Gitlab::Markdown::ReferenceFilter.name) + doc = filter(link, current_user: user) + + expect(doc.css('a').length).to eq 0 + end + + it 'allows permitted Project references' do + user = create(:user) + project = create(:empty_project) + project.team << [user, :master] + + link = reference_link(project: project.id, reference_filter: Gitlab::Markdown::ReferenceFilter.name) + doc = filter(link, current_user: user) + + expect(doc.css('a').length).to eq 1 + end + + it 'handles invalid Project references' do + link = reference_link(project: 12345, reference_filter: Gitlab::Markdown::ReferenceFilter.name) + + expect { filter(link) }.not_to raise_error + end + end + + context "for user references" do + + context 'with data-group' do + it 'removes unpermitted Group references' do + user = create(:user) + group = create(:group) + + link = reference_link(group: group.id, reference_filter: Gitlab::Markdown::UserReferenceFilter.name) + doc = filter(link, current_user: user) + + expect(doc.css('a').length).to eq 0 + end + + it 'allows permitted Group references' do + user = create(:user) + group = create(:group) + group.add_developer(user) + + link = reference_link(group: group.id, reference_filter: Gitlab::Markdown::UserReferenceFilter.name) + doc = filter(link, current_user: user) + + expect(doc.css('a').length).to eq 1 + end + + it 'handles invalid Group references' do + link = reference_link(group: 12345, reference_filter: Gitlab::Markdown::UserReferenceFilter.name) + + expect { filter(link) }.not_to raise_error + end + end + + context 'with data-user' do + it 'allows any User reference' do + user = create(:user) + + link = reference_link(user: user.id, reference_filter: Gitlab::Markdown::UserReferenceFilter.name) + doc = filter(link) + + expect(doc.css('a').length).to eq 1 + end + end + end + end +end diff --git a/spec/lib/gitlab/markdown/reference_gatherer_filter_spec.rb b/spec/lib/gitlab/markdown/reference_gatherer_filter_spec.rb new file mode 100644 index 00000000000..4fa473ad191 --- /dev/null +++ b/spec/lib/gitlab/markdown/reference_gatherer_filter_spec.rb @@ -0,0 +1,89 @@ +require 'spec_helper' + +module Gitlab::Markdown + describe ReferenceGathererFilter do + include ActionView::Helpers::UrlHelper + include FilterSpecHelper + + def reference_link(data) + link_to('text', '', class: 'gfm', data: data) + end + + context "for issue references" do + + context 'with data-project' do + it 'removes unpermitted Project references' do + user = create(:user) + project = create(:empty_project) + issue = create(:issue, project: project) + + link = reference_link(project: project.id, issue: issue.id, reference_filter: Gitlab::Markdown::IssueReferenceFilter.name) + result = pipeline_result(link, current_user: user) + + expect(result[:references][:issue]).to be_empty + end + + it 'allows permitted Project references' do + user = create(:user) + project = create(:empty_project) + issue = create(:issue, project: project) + project.team << [user, :master] + + link = reference_link(project: project.id, issue: issue.id, reference_filter: Gitlab::Markdown::IssueReferenceFilter.name) + result = pipeline_result(link, current_user: user) + + expect(result[:references][:issue]).to eq([issue]) + end + + it 'handles invalid Project references' do + link = reference_link(project: 12345, issue: 12345, reference_filter: Gitlab::Markdown::IssueReferenceFilter.name) + + expect { pipeline_result(link) }.not_to raise_error + end + end + end + + context "for user references" do + + context 'with data-group' do + it 'removes unpermitted Group references' do + user = create(:user) + group = create(:group) + + link = reference_link(group: group.id, reference_filter: Gitlab::Markdown::UserReferenceFilter.name) + result = pipeline_result(link, current_user: user) + + expect(result[:references][:user]).to be_empty + end + + it 'allows permitted Group references' do + user = create(:user) + group = create(:group) + group.add_developer(user) + + link = reference_link(group: group.id, reference_filter: Gitlab::Markdown::UserReferenceFilter.name) + result = pipeline_result(link, current_user: user) + + expect(result[:references][:user]).to eq([user]) + end + + it 'handles invalid Group references' do + link = reference_link(group: 12345, reference_filter: Gitlab::Markdown::UserReferenceFilter.name) + + expect { pipeline_result(link) }.not_to raise_error + end + end + + context 'with data-user' do + it 'allows any User reference' do + user = create(:user) + + link = reference_link(user: user.id, reference_filter: Gitlab::Markdown::UserReferenceFilter.name) + result = pipeline_result(link) + + expect(result[:references][:user]).to eq([user]) + end + end + end + end +end diff --git a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb index e50c82d0b3c..27cd00e8054 100644 --- a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb +++ b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb @@ -44,7 +44,7 @@ module Gitlab::Markdown instance = described_class.new('Foo') 3.times { instance.whitelist } - expect(instance.whitelist[:transformers].size).to eq 4 + expect(instance.whitelist[:transformers].size).to eq 5 end it 'allows syntax highlighting' do @@ -77,19 +77,100 @@ module Gitlab::Markdown end it 'removes `rel` attribute from `a` elements' do - doc = filter(%q{<a href="#" rel="nofollow">Link</a>}) + act = %q{<a href="#" rel="nofollow">Link</a>} + exp = %q{<a href="#">Link</a>} - expect(doc.css('a').size).to eq 1 - expect(doc.at_css('a')['href']).to eq '#' - expect(doc.at_css('a')['rel']).to be_nil + expect(filter(act).to_html).to eq exp end - it 'removes script-like `href` attribute from `a` elements' do - html = %q{<a href="javascript:alert('Hi')">Hi</a>} - doc = filter(html) + # Adapted from the Sanitize test suite: http://git.io/vczrM + protocols = { + 'protocol-based JS injection: simple, no spaces' => { + input: '<a href="javascript:alert(\'XSS\');">foo</a>', + output: '<a>foo</a>' + }, + + 'protocol-based JS injection: simple, spaces before' => { + input: '<a href="javascript :alert(\'XSS\');">foo</a>', + output: '<a>foo</a>' + }, + + 'protocol-based JS injection: simple, spaces after' => { + input: '<a href="javascript: alert(\'XSS\');">foo</a>', + output: '<a>foo</a>' + }, + + 'protocol-based JS injection: simple, spaces before and after' => { + input: '<a href="javascript : alert(\'XSS\');">foo</a>', + output: '<a>foo</a>' + }, + + 'protocol-based JS injection: preceding colon' => { + input: '<a href=":javascript:alert(\'XSS\');">foo</a>', + output: '<a>foo</a>' + }, + + 'protocol-based JS injection: UTF-8 encoding' => { + input: '<a href="javascript:">foo</a>', + output: '<a>foo</a>' + }, + + 'protocol-based JS injection: long UTF-8 encoding' => { + input: '<a href="javascript:">foo</a>', + output: '<a>foo</a>' + }, + + 'protocol-based JS injection: long UTF-8 encoding without semicolons' => { + input: '<a href=javascript:alert('XSS')>foo</a>', + output: '<a>foo</a>' + }, + + 'protocol-based JS injection: hex encoding' => { + input: '<a href="javascript:">foo</a>', + output: '<a>foo</a>' + }, + + 'protocol-based JS injection: long hex encoding' => { + input: '<a href="javascript:">foo</a>', + output: '<a>foo</a>' + }, + + 'protocol-based JS injection: hex encoding without semicolons' => { + input: '<a href=javascript:alert('XSS')>foo</a>', + output: '<a>foo</a>' + }, + + 'protocol-based JS injection: null char' => { + input: "<a href=java\0script:alert(\"XSS\")>foo</a>", + output: '<a href="java"></a>' + }, + + 'protocol-based JS injection: spaces and entities' => { + input: '<a href="  javascript:alert(\'XSS\');">foo</a>', + output: '<a href="">foo</a>' + }, + } + + protocols.each do |name, data| + it "handles #{name}" do + doc = filter(data[:input]) + + expect(doc.to_html).to eq data[:output] + end + end + + it 'allows non-standard anchor schemes' do + exp = %q{<a href="irc://irc.freenode.net/git">IRC</a>} + act = filter(exp) + + expect(act.to_html).to eq exp + end + + it 'allows relative links' do + exp = %q{<a href="foo/bar.md">foo/bar.md</a>} + act = filter(exp) - expect(doc.css('a').size).to eq 1 - expect(doc.at_css('a')['href']).to be_nil + expect(act.to_html).to eq exp end end diff --git a/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb b/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb index fd3f0d20fad..9d9652dba46 100644 --- a/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb @@ -4,7 +4,7 @@ module Gitlab::Markdown describe SnippetReferenceFilter do include FilterSpecHelper - let(:project) { create(:empty_project) } + let(:project) { create(:empty_project, :public) } let(:snippet) { create(:project_snippet, project: project) } let(:reference) { snippet.to_reference } @@ -55,12 +55,20 @@ module Gitlab::Markdown expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-snippet' end - it 'includes a data-project-id attribute' do + it 'includes a data-project attribute' do doc = filter("Snippet #{reference}") link = doc.css('a').first - expect(link).to have_attribute('data-project-id') - expect(link.attr('data-project-id')).to eq project.id.to_s + expect(link).to have_attribute('data-project') + expect(link.attr('data-project')).to eq project.id.to_s + end + + it 'includes a data-snippet attribute' do + doc = filter("See #{reference}") + link = doc.css('a').first + + expect(link).to have_attribute('data-snippet') + expect(link.attr('data-snippet')).to eq snippet.id.to_s end it 'supports an :only_path context' do @@ -72,52 +80,38 @@ module Gitlab::Markdown end it 'adds to the results hash' do - result = pipeline_result("Snippet #{reference}") + result = reference_pipeline_result("Snippet #{reference}") expect(result[:references][:snippet]).to eq [snippet] end end context 'cross-project reference' do let(:namespace) { create(:namespace, name: 'cross-reference') } - let(:project2) { create(:empty_project, namespace: namespace) } + let(:project2) { create(:empty_project, :public, namespace: namespace) } let(:snippet) { create(:project_snippet, project: project2) } let(:reference) { snippet.to_reference(project) } - context 'when user can access reference' do - before { allow_cross_reference! } - - it 'links to a valid reference' do - doc = filter("See #{reference}") - - expect(doc.css('a').first.attr('href')). - to eq urls.namespace_project_snippet_url(project2.namespace, project2, snippet) - end - - it 'links with adjacent text' do - doc = filter("See (#{reference}.)") - expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/) - end - - it 'ignores invalid snippet IDs on the referenced project' do - exp = act = "See #{invalidate_reference(reference)}" + it 'links to a valid reference' do + doc = filter("See #{reference}") - expect(filter(act).to_html).to eq exp - end + expect(doc.css('a').first.attr('href')). + to eq urls.namespace_project_snippet_url(project2.namespace, project2, snippet) + end - it 'adds to the results hash' do - result = pipeline_result("Snippet #{reference}") - expect(result[:references][:snippet]).to eq [snippet] - end + it 'links with adjacent text' do + doc = filter("See (#{reference}.)") + expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/) end - context 'when user cannot access reference' do - before { disallow_cross_reference! } + it 'ignores invalid snippet IDs on the referenced project' do + exp = act = "See #{invalidate_reference(reference)}" - it 'ignores valid references' do - exp = act = "See #{reference}" + expect(filter(act).to_html).to eq exp + end - expect(filter(act).to_html).to eq exp - end + it 'adds to the results hash' do + result = reference_pipeline_result("Snippet #{reference}") + expect(result[:references][:snippet]).to eq [snippet] end end end diff --git a/spec/lib/gitlab/markdown/upload_link_filter_spec.rb b/spec/lib/gitlab/markdown/upload_link_filter_spec.rb new file mode 100644 index 00000000000..9ae45a6f559 --- /dev/null +++ b/spec/lib/gitlab/markdown/upload_link_filter_spec.rb @@ -0,0 +1,75 @@ +# encoding: UTF-8 + +require 'spec_helper' + +module Gitlab::Markdown + describe UploadLinkFilter do + def filter(doc, contexts = {}) + contexts.reverse_merge!({ + project: project + }) + + described_class.call(doc, contexts) + end + + def image(path) + %(<img src="#{path}" />) + end + + def link(path) + %(<a href="#{path}">#{path}</a>) + end + + let(:project) { create(:project) } + + shared_examples :preserve_unchanged do + it 'does not modify any relative URL in anchor' do + doc = filter(link('README.md')) + expect(doc.at_css('a')['href']).to eq 'README.md' + end + + it 'does not modify any relative URL in image' do + doc = filter(image('files/images/logo-black.png')) + expect(doc.at_css('img')['src']).to eq 'files/images/logo-black.png' + end + end + + it 'does not raise an exception on invalid URIs' do + act = link("://foo") + expect { filter(act) }.not_to raise_error + end + + context 'with a valid repository' do + it 'rebuilds relative URL for a link' do + doc = filter(link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg')) + expect(doc.at_css('a')['href']). + to eq "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg" + end + + it 'rebuilds relative URL for an image' do + doc = filter(link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg')) + expect(doc.at_css('a')['href']). + to eq "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg" + end + + it 'does not modify absolute URL' do + doc = filter(link('http://example.com')) + expect(doc.at_css('a')['href']).to eq 'http://example.com' + end + + it 'supports Unicode filenames' do + path = '/uploads/한글.png' + escaped = Addressable::URI.escape(path) + + # Stub these methods so the file doesn't actually need to be in the repo + allow_any_instance_of(described_class). + to receive(:file_exists?).and_return(true) + allow_any_instance_of(described_class). + to receive(:image?).with(path).and_return(true) + + doc = filter(image(escaped)) + expect(doc.at_css('img')['src']).to match "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/%ED%95%9C%EA%B8%80.png" + end + end + end +end diff --git a/spec/lib/gitlab/markdown/user_reference_filter_spec.rb b/spec/lib/gitlab/markdown/user_reference_filter_spec.rb index b2155fab59b..d9e0d7c42db 100644 --- a/spec/lib/gitlab/markdown/user_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/user_reference_filter_spec.rb @@ -4,7 +4,7 @@ module Gitlab::Markdown describe UserReferenceFilter do include FilterSpecHelper - let(:project) { create(:empty_project) } + let(:project) { create(:empty_project, :public) } let(:user) { create(:user) } let(:reference) { user.to_reference } @@ -39,7 +39,7 @@ module Gitlab::Markdown end it 'adds to the results hash' do - result = pipeline_result("Hey #{reference}") + result = reference_pipeline_result("Hey #{reference}") expect(result[:references][:user]).to eq [project.creator] end end @@ -64,59 +64,40 @@ module Gitlab::Markdown expect(doc.css('a').length).to eq 1 end - it 'includes a data-user-id attribute' do + it 'includes a data-user attribute' do doc = filter("Hey #{reference}") link = doc.css('a').first - expect(link).to have_attribute('data-user-id') - expect(link.attr('data-user-id')).to eq user.namespace.owner_id.to_s + expect(link).to have_attribute('data-user') + expect(link.attr('data-user')).to eq user.namespace.owner_id.to_s end it 'adds to the results hash' do - result = pipeline_result("Hey #{reference}") + result = reference_pipeline_result("Hey #{reference}") expect(result[:references][:user]).to eq [user] end end context 'mentioning a group' do let(:group) { create(:group) } - let(:user) { create(:user) } let(:reference) { group.to_reference } - context 'that the current user can read' do - before do - group.add_developer(user) - end - - it 'links to the Group' do - doc = filter("Hey #{reference}", current_user: user) - expect(doc.css('a').first.attr('href')).to eq urls.group_url(group) - end - - it 'includes a data-group-id attribute' do - doc = filter("Hey #{reference}", current_user: user) - link = doc.css('a').first + it 'links to the Group' do + doc = filter("Hey #{reference}") + expect(doc.css('a').first.attr('href')).to eq urls.group_url(group) + end - expect(link).to have_attribute('data-group-id') - expect(link.attr('data-group-id')).to eq group.id.to_s - end + it 'includes a data-group attribute' do + doc = filter("Hey #{reference}") + link = doc.css('a').first - it 'adds to the results hash' do - result = pipeline_result("Hey #{reference}", current_user: user) - expect(result[:references][:user]).to eq group.users - end + expect(link).to have_attribute('data-group') + expect(link.attr('data-group')).to eq group.id.to_s end - context 'that the current user cannot read' do - it 'ignores references to the Group' do - doc = filter("Hey #{reference}", current_user: user) - expect(doc.to_html).to eq "Hey #{reference}" - end - - it 'does not add to the results hash' do - result = pipeline_result("Hey #{reference}", current_user: user) - expect(result[:references][:user]).to eq [] - end + it 'adds to the results hash' do + result = reference_pipeline_result("Hey #{reference}") + expect(result[:references][:user]).to eq group.users end end diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb index 32a25f08cac..19327ac8ce0 100644 --- a/spec/lib/gitlab/project_search_results_spec.rb +++ b/spec/lib/gitlab/project_search_results_spec.rb @@ -9,7 +9,7 @@ describe Gitlab::ProjectSearchResults do it { expect(results.project).to eq(project) } it { expect(results.repository_ref).to be_nil } - it { expect(results.query).to eq('hello\\ world') } + it { expect(results.query).to eq('hello world') } end describe 'initialize with ref' do @@ -18,6 +18,6 @@ describe Gitlab::ProjectSearchResults do it { expect(results.project).to eq(project) } it { expect(results.repository_ref).to eq(ref) } - it { expect(results.query).to eq('hello\\ world') } + it { expect(results.query).to eq('hello world') } end end diff --git a/spec/lib/gitlab/push_data_builder_spec.rb b/spec/lib/gitlab/push_data_builder_spec.rb index 1b8ba7b4d43..02710742625 100644 --- a/spec/lib/gitlab/push_data_builder_spec.rb +++ b/spec/lib/gitlab/push_data_builder_spec.rb @@ -17,6 +17,9 @@ describe 'Gitlab::PushDataBuilder' do it { expect(data[:repository][:git_ssh_url]).to eq(project.ssh_url_to_repo) } it { expect(data[:repository][:visibility_level]).to eq(project.visibility_level) } it { expect(data[:total_commits_count]).to eq(3) } + it { expect(data[:added]).to eq(["gitlab-grack"]) } + it { expect(data[:modified]).to eq([".gitmodules", "files/ruby/popen.rb", "files/ruby/regex.rb"]) } + it { expect(data[:removed]).to eq([]) } end describe :build do @@ -35,5 +38,8 @@ describe 'Gitlab::PushDataBuilder' do it { expect(data[:ref]).to eq('refs/tags/v1.1.0') } it { expect(data[:commits]).to be_empty } it { expect(data[:total_commits_count]).to be_zero } + it { expect(data[:added]).to eq([]) } + it { expect(data[:modified]).to eq([]) } + it { expect(data[:removed]).to eq([]) } end end diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index 088e34f050c..ad84d2274e8 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -13,7 +13,7 @@ describe Gitlab::ReferenceExtractor do project.team << [@u_bar, :guest] subject.analyze('@foo, @baduser, @bar, and @offteam') - expect(subject.users).to eq([@u_foo, @u_bar, @u_offteam]) + expect(subject.users).to match_array([@u_foo, @u_bar, @u_offteam]) end it 'ignores user mentions inside specific elements' do @@ -37,7 +37,7 @@ describe Gitlab::ReferenceExtractor do > @offteam }) - expect(subject.users).to eq([]) + expect(subject.users).to match_array([]) end it 'accesses valid issue objects' do @@ -45,7 +45,7 @@ describe Gitlab::ReferenceExtractor do @i1 = create(:issue, project: project) subject.analyze("#{@i0.to_reference}, #{@i1.to_reference}, and #{Issue.reference_prefix}999.") - expect(subject.issues).to eq([@i0, @i1]) + expect(subject.issues).to match_array([@i0, @i1]) end it 'accesses valid merge requests' do @@ -53,7 +53,7 @@ describe Gitlab::ReferenceExtractor do @m1 = create(:merge_request, source_project: project, target_project: project, source_branch: 'feature_conflict') subject.analyze("!999, !#{@m1.iid}, and !#{@m0.iid}.") - expect(subject.merge_requests).to eq([@m1, @m0]) + expect(subject.merge_requests).to match_array([@m1, @m0]) end it 'accesses valid labels' do @@ -62,7 +62,7 @@ describe Gitlab::ReferenceExtractor do @l2 = create(:label) subject.analyze("~#{@l0.id}, ~999, ~#{@l2.id}, ~#{@l1.id}") - expect(subject.labels).to eq([@l0, @l1]) + expect(subject.labels).to match_array([@l0, @l1]) end it 'accesses valid snippets' do @@ -71,7 +71,7 @@ describe Gitlab::ReferenceExtractor do @s2 = create(:project_snippet) subject.analyze("$#{@s0.id}, $999, $#{@s2.id}, $#{@s1.id}") - expect(subject.snippets).to eq([@s0, @s1]) + expect(subject.snippets).to match_array([@s0, @s1]) end it 'accesses valid commits' do @@ -109,7 +109,7 @@ describe Gitlab::ReferenceExtractor do subject.analyze("this refers issue #{issue.to_reference(project)}") extracted = subject.issues expect(extracted.size).to eq(1) - expect(extracted).to eq([issue]) + expect(extracted).to match_array([issue]) end end end diff --git a/spec/lib/gitlab/sherlock/collection_spec.rb b/spec/lib/gitlab/sherlock/collection_spec.rb new file mode 100644 index 00000000000..a8a9d6fc7bc --- /dev/null +++ b/spec/lib/gitlab/sherlock/collection_spec.rb @@ -0,0 +1,82 @@ +require 'spec_helper' + +describe Gitlab::Sherlock::Collection do + let(:collection) { described_class.new } + + let(:transaction) do + Gitlab::Sherlock::Transaction.new('POST', '/cat_pictures') + end + + describe '#add' do + it 'adds a new transaction' do + collection.add(transaction) + + expect(collection).to_not be_empty + end + + it 'is aliased as <<' do + collection << transaction + + expect(collection).to_not be_empty + end + end + + describe '#each' do + it 'iterates over every transaction' do + collection.add(transaction) + + expect { |b| collection.each(&b) }.to yield_with_args(transaction) + end + end + + describe '#clear' do + it 'removes all transactions' do + collection.add(transaction) + + collection.clear + + expect(collection).to be_empty + end + end + + describe '#empty?' do + it 'returns true for an empty collection' do + expect(collection).to be_empty + end + + it 'returns false for a collection with a transaction' do + collection.add(transaction) + + expect(collection).to_not be_empty + end + end + + describe '#find_transaction' do + it 'returns the transaction for the given ID' do + collection.add(transaction) + + expect(collection.find_transaction(transaction.id)).to eq(transaction) + end + + it 'returns nil when no transaction could be found' do + collection.add(transaction) + + expect(collection.find_transaction('cats')).to be_nil + end + end + + describe '#newest_first' do + it 'returns transactions sorted from new to old' do + trans1 = Gitlab::Sherlock::Transaction.new('POST', '/cat_pictures') + trans2 = Gitlab::Sherlock::Transaction.new('POST', '/more_cat_pictures') + + allow(trans1).to receive(:finished_at).and_return(Time.utc(2015, 1, 1)) + allow(trans2).to receive(:finished_at).and_return(Time.utc(2015, 1, 2)) + + collection.add(trans1) + collection.add(trans2) + + expect(collection.newest_first).to eq([trans2, trans1]) + end + end +end diff --git a/spec/lib/gitlab/sherlock/file_sample_spec.rb b/spec/lib/gitlab/sherlock/file_sample_spec.rb new file mode 100644 index 00000000000..f05a59f56f6 --- /dev/null +++ b/spec/lib/gitlab/sherlock/file_sample_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +describe Gitlab::Sherlock::FileSample do + let(:sample) { described_class.new(__FILE__, [], 150.4, 2) } + + describe '#id' do + it 'returns the ID' do + expect(sample.id).to be_an_instance_of(String) + end + end + + describe '#file' do + it 'returns the file path' do + expect(sample.file).to eq(__FILE__) + end + end + + describe '#line_samples' do + it 'returns the line samples' do + expect(sample.line_samples).to eq([]) + end + end + + describe '#events' do + it 'returns the total number of events' do + expect(sample.events).to eq(2) + end + end + + describe '#duration' do + it 'returns the total execution time' do + expect(sample.duration).to eq(150.4) + end + end + + describe '#relative_path' do + it 'returns the relative path' do + expect(sample.relative_path). + to eq('spec/lib/gitlab/sherlock/file_sample_spec.rb') + end + end + + describe '#to_param' do + it 'returns the sample ID' do + expect(sample.to_param).to eq(sample.id) + end + end + + describe '#source' do + it 'returns the contents of the file' do + expect(sample.source).to eq(File.read(__FILE__)) + end + end +end diff --git a/spec/lib/gitlab/sherlock/line_profiler_spec.rb b/spec/lib/gitlab/sherlock/line_profiler_spec.rb new file mode 100644 index 00000000000..8f2e1299714 --- /dev/null +++ b/spec/lib/gitlab/sherlock/line_profiler_spec.rb @@ -0,0 +1,73 @@ +require 'spec_helper' + +describe Gitlab::Sherlock::LineProfiler do + let(:profiler) { described_class.new } + + describe '#profile' do + it 'runs the profiler when using MRI' do + allow(profiler).to receive(:mri?).and_return(true) + allow(profiler).to receive(:profile_mri) + + profiler.profile { 'cats' } + end + + it 'raises NotImplementedError when profiling an unsupported platform' do + allow(profiler).to receive(:mri?).and_return(false) + + expect { profiler.profile { 'cats' } }.to raise_error(NotImplementedError) + end + end + + describe '#profile_mri' do + it 'returns an Array containing the return value and profiling samples' do + allow(profiler).to receive(:lineprof). + and_yield. + and_return({ __FILE__ => [[0, 0, 0, 0]] }) + + retval, samples = profiler.profile_mri { 42 } + + expect(retval).to eq(42) + expect(samples).to eq([]) + end + end + + describe '#aggregate_rblineprof' do + let(:raw_samples) do + { __FILE__ => [[30000, 30000, 5, 0], [15000, 15000, 4, 0]] } + end + + it 'returns an Array of FileSample objects' do + samples = profiler.aggregate_rblineprof(raw_samples) + + expect(samples).to be_an_instance_of(Array) + expect(samples[0]).to be_an_instance_of(Gitlab::Sherlock::FileSample) + end + + describe 'the first FileSample object' do + let(:file_sample) do + profiler.aggregate_rblineprof(raw_samples)[0] + end + + it 'uses the correct file path' do + expect(file_sample.file).to eq(__FILE__) + end + + it 'contains a list of line samples' do + line_sample = file_sample.line_samples[0] + + expect(line_sample).to be_an_instance_of(Gitlab::Sherlock::LineSample) + + expect(line_sample.duration).to eq(15.0) + expect(line_sample.events).to eq(4) + end + + it 'contains the total file execution time' do + expect(file_sample.duration).to eq(30.0) + end + + it 'contains the total amount of file events' do + expect(file_sample.events).to eq(5) + end + end + end +end diff --git a/spec/lib/gitlab/sherlock/line_sample_spec.rb b/spec/lib/gitlab/sherlock/line_sample_spec.rb new file mode 100644 index 00000000000..5f02f6a3213 --- /dev/null +++ b/spec/lib/gitlab/sherlock/line_sample_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe Gitlab::Sherlock::LineSample do + let(:sample) { described_class.new(150.0, 4) } + + describe '#duration' do + it 'returns the duration' do + expect(sample.duration).to eq(150.0) + end + end + + describe '#events' do + it 'returns the amount of events' do + expect(sample.events).to eq(4) + end + end + + describe '#percentage_of' do + it 'returns the percentage of 1500.0' do + expect(sample.percentage_of(1500.0)).to be_within(0.1).of(10.0) + end + end + + describe '#majority_of' do + it 'returns true if the sample takes up the majority of the given duration' do + expect(sample.majority_of?(500.0)).to eq(true) + end + + it "returns false if the sample doesn't take up the majority of the given duration" do + expect(sample.majority_of?(1500.0)).to eq(false) + end + end +end diff --git a/spec/lib/gitlab/sherlock/location_spec.rb b/spec/lib/gitlab/sherlock/location_spec.rb new file mode 100644 index 00000000000..b295a624b35 --- /dev/null +++ b/spec/lib/gitlab/sherlock/location_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +describe Gitlab::Sherlock::Location do + let(:location) { described_class.new(__FILE__, 1) } + + describe 'from_ruby_location' do + it 'creates a Location from a Thread::Backtrace::Location' do + input = caller_locations[0] + output = described_class.from_ruby_location(input) + + expect(output).to be_an_instance_of(described_class) + expect(output.path).to eq(input.path) + expect(output.line).to eq(input.lineno) + end + end + + describe '#path' do + it 'returns the file path' do + expect(location.path).to eq(__FILE__) + end + end + + describe '#line' do + it 'returns the line number' do + expect(location.line).to eq(1) + end + end + + describe '#application?' do + it 'returns true for an application frame' do + expect(location.application?).to eq(true) + end + + it 'returns false for a non application frame' do + loc = described_class.new('/tmp/cats.rb', 1) + + expect(loc.application?).to eq(false) + end + end +end diff --git a/spec/lib/gitlab/sherlock/middleware_spec.rb b/spec/lib/gitlab/sherlock/middleware_spec.rb new file mode 100644 index 00000000000..aa74fc53a79 --- /dev/null +++ b/spec/lib/gitlab/sherlock/middleware_spec.rb @@ -0,0 +1,79 @@ +require 'spec_helper' + +describe Gitlab::Sherlock::Middleware do + let(:app) { double(:app) } + let(:middleware) { described_class.new(app) } + + describe '#call' do + describe 'when instrumentation is enabled' do + it 'instruments a request' do + allow(middleware).to receive(:instrument?).and_return(true) + allow(middleware).to receive(:call_with_instrumentation) + + middleware.call({}) + end + end + + describe 'when instrumentation is disabled' do + it "doesn't instrument a request" do + allow(middleware).to receive(:instrument).and_return(false) + allow(app).to receive(:call) + + middleware.call({}) + end + end + end + + describe '#call_with_instrumentation' do + it 'instruments a request' do + trans = double(:transaction) + retval = 'cats are amazing' + env = {} + + allow(app).to receive(:call).with(env).and_return(retval) + allow(middleware).to receive(:transaction_from_env).and_return(trans) + allow(trans).to receive(:run).and_yield.and_return(retval) + allow(Gitlab::Sherlock.collection).to receive(:add).with(trans) + + middleware.call_with_instrumentation(env) + end + end + + describe '#instrument?' do + it 'returns false for a text/css request' do + env = { 'HTTP_ACCEPT' => 'text/css', 'REQUEST_URI' => '/' } + + expect(middleware.instrument?(env)).to eq(false) + end + + it 'returns false for a request to a Sherlock route' do + env = { + 'HTTP_ACCEPT' => 'text/html', + 'REQUEST_URI' => '/sherlock/transactions' + } + + expect(middleware.instrument?(env)).to eq(false) + end + + it 'returns true for a request that should be instrumented' do + env = { + 'HTTP_ACCEPT' => 'text/html', + 'REQUEST_URI' => '/cats' + } + + expect(middleware.instrument?(env)).to eq(true) + end + end + + describe '#transaction_from_env' do + it 'returns a Transaction' do + env = { + 'HTTP_ACCEPT' => 'text/html', + 'REQUEST_URI' => '/cats' + } + + expect(middleware.transaction_from_env(env)). + to be_an_instance_of(Gitlab::Sherlock::Transaction) + end + end +end diff --git a/spec/lib/gitlab/sherlock/query_spec.rb b/spec/lib/gitlab/sherlock/query_spec.rb new file mode 100644 index 00000000000..a9afef5dc1d --- /dev/null +++ b/spec/lib/gitlab/sherlock/query_spec.rb @@ -0,0 +1,113 @@ +require 'spec_helper' + +describe Gitlab::Sherlock::Query do + let(:started_at) { Time.utc(2015, 1, 1) } + let(:finished_at) { started_at + 5 } + + let(:query) do + described_class.new('SELECT COUNT(*) FROM users', started_at, finished_at) + end + + describe 'new_with_bindings' do + it 'returns a Query' do + sql = 'SELECT COUNT(*) FROM users WHERE id = $1' + bindings = [[double(:column), 10]] + + query = described_class. + new_with_bindings(sql, bindings, started_at, finished_at) + + expect(query.query).to eq('SELECT COUNT(*) FROM users WHERE id = 10;') + end + end + + describe '#id' do + it 'returns a String' do + expect(query.id).to be_an_instance_of(String) + end + end + + describe '#query' do + it 'returns the query with a trailing semi-colon' do + expect(query.query).to eq('SELECT COUNT(*) FROM users;') + end + end + + describe '#started_at' do + it 'returns the start time' do + expect(query.started_at).to eq(started_at) + end + end + + describe '#finished_at' do + it 'returns the completion time' do + expect(query.finished_at).to eq(finished_at) + end + end + + describe '#backtrace' do + it 'returns the backtrace' do + expect(query.backtrace).to be_an_instance_of(Array) + end + end + + describe '#duration' do + it 'returns the duration in milliseconds' do + expect(query.duration).to be_within(0.1).of(5000.0) + end + end + + describe '#to_param' do + it 'returns the query ID' do + expect(query.to_param).to eq(query.id) + end + end + + describe '#formatted_query' do + it 'returns a formatted version of the query' do + expect(query.formatted_query).to eq(<<-EOF.strip) +SELECT COUNT(*) +FROM users; + EOF + end + end + + describe '#last_application_frame' do + it 'returns the last application frame' do + frame = query.last_application_frame + + expect(frame).to be_an_instance_of(Gitlab::Sherlock::Location) + expect(frame.path).to eq(__FILE__) + end + end + + describe '#application_backtrace' do + it 'returns an Array of application frames' do + frames = query.application_backtrace + + expect(frames).to be_an_instance_of(Array) + expect(frames).to_not be_empty + + frames.each do |frame| + expect(frame.path).to start_with(Rails.root.to_s) + end + end + end + + describe '#explain' do + it 'returns the query plan as a String' do + lines = [ + ['Aggregate (cost=123 rows=1)'], + [' -> Index Only Scan using index_cats_are_amazing'] + ] + + result = double(:result, values: lines) + + allow(query).to receive(:raw_explain).and_return(result) + + expect(query.explain).to eq(<<-EOF.strip) +Aggregate (cost=123 rows=1) + -> Index Only Scan using index_cats_are_amazing + EOF + end + end +end diff --git a/spec/lib/gitlab/sherlock/transaction_spec.rb b/spec/lib/gitlab/sherlock/transaction_spec.rb new file mode 100644 index 00000000000..bb49fb65cf8 --- /dev/null +++ b/spec/lib/gitlab/sherlock/transaction_spec.rb @@ -0,0 +1,222 @@ +require 'spec_helper' + +describe Gitlab::Sherlock::Transaction do + let(:transaction) { described_class.new('POST', '/cat_pictures') } + + describe '#id' do + it 'returns the transaction ID' do + expect(transaction.id).to be_an_instance_of(String) + end + end + + describe '#type' do + it 'returns the type' do + expect(transaction.type).to eq('POST') + end + end + + describe '#path' do + it 'returns the path' do + expect(transaction.path).to eq('/cat_pictures') + end + end + + describe '#queries' do + it 'returns an Array of queries' do + expect(transaction.queries).to be_an_instance_of(Array) + end + end + + describe '#file_samples' do + it 'returns an Array of file samples' do + expect(transaction.file_samples).to be_an_instance_of(Array) + end + end + + describe '#started_at' do + it 'returns the start time' do + allow(transaction).to receive(:profile_lines).and_yield + + transaction.run { 'cats are amazing' } + + expect(transaction.started_at).to be_an_instance_of(Time) + end + end + + describe '#finished_at' do + it 'returns the completion time' do + allow(transaction).to receive(:profile_lines).and_yield + + transaction.run { 'cats are amazing' } + + expect(transaction.finished_at).to be_an_instance_of(Time) + end + end + + describe '#view_counts' do + it 'returns a Hash' do + expect(transaction.view_counts).to be_an_instance_of(Hash) + end + + it 'sets the default value of a key to 0' do + expect(transaction.view_counts['cats.rb']).to be_zero + end + end + + describe '#run' do + it 'runs the transaction' do + allow(transaction).to receive(:profile_lines).and_yield + + retval = transaction.run { 'cats are amazing' } + + expect(retval).to eq('cats are amazing') + end + end + + describe '#duration' do + it 'returns the duration in seconds' do + start_time = Time.now + + allow(transaction).to receive(:started_at).and_return(start_time) + allow(transaction).to receive(:finished_at).and_return(start_time + 5) + + expect(transaction.duration).to be_within(0.1).of(5.0) + end + end + + describe '#to_param' do + it 'returns the transaction ID' do + expect(transaction.to_param).to eq(transaction.id) + end + end + + describe '#sorted_queries' do + it 'returns the queries in descending order' do + start_time = Time.now + + query1 = Gitlab::Sherlock::Query.new('SELECT 1', start_time, start_time) + + query2 = Gitlab::Sherlock::Query. + new('SELECT 2', start_time, start_time + 5) + + transaction.queries << query1 + transaction.queries << query2 + + expect(transaction.sorted_queries).to eq([query2, query1]) + end + end + + describe '#sorted_file_samples' do + it 'returns the file samples in descending order' do + sample1 = Gitlab::Sherlock::FileSample.new(__FILE__, [], 10.0, 1) + sample2 = Gitlab::Sherlock::FileSample.new(__FILE__, [], 15.0, 1) + + transaction.file_samples << sample1 + transaction.file_samples << sample2 + + expect(transaction.sorted_file_samples).to eq([sample2, sample1]) + end + end + + describe '#find_query' do + it 'returns a Query when found' do + query = Gitlab::Sherlock::Query.new('SELECT 1', Time.now, Time.now) + + transaction.queries << query + + expect(transaction.find_query(query.id)).to eq(query) + end + + it 'returns nil when no query could be found' do + expect(transaction.find_query('cats')).to be_nil + end + end + + describe '#find_file_sample' do + it 'returns a FileSample when found' do + sample = Gitlab::Sherlock::FileSample.new(__FILE__, [], 10.0, 1) + + transaction.file_samples << sample + + expect(transaction.find_file_sample(sample.id)).to eq(sample) + end + + it 'returns nil when no file sample could be found' do + expect(transaction.find_file_sample('cats')).to be_nil + end + end + + describe '#profile_lines' do + describe 'when line profiling is enabled' do + it 'yields the block using the line profiler' do + allow(Gitlab::Sherlock).to receive(:enable_line_profiler?). + and_return(true) + + allow_any_instance_of(Gitlab::Sherlock::LineProfiler). + to receive(:profile).and_return('cats are amazing', []) + + retval = transaction.profile_lines { 'cats are amazing' } + + expect(retval).to eq('cats are amazing') + end + end + + describe 'when line profiling is disabled' do + it 'yields the block' do + allow(Gitlab::Sherlock).to receive(:enable_line_profiler?). + and_return(false) + + retval = transaction.profile_lines { 'cats are amazing' } + + expect(retval).to eq('cats are amazing') + end + end + end + + describe '#subscribe_to_active_record' do + let(:subscription) { transaction.subscribe_to_active_record } + let(:time) { Time.now } + let(:query_data) { { sql: 'SELECT 1', binds: [] } } + + after do + ActiveSupport::Notifications.unsubscribe(subscription) + end + + it 'tracks executed queries' do + expect(transaction).to receive(:track_query). + with('SELECT 1', [], time, time) + + subscription.publish('test', time, time, nil, query_data) + end + + it 'only tracks queries triggered from the transaction thread' do + expect(transaction).to_not receive(:track_query) + + Thread.new { subscription.publish('test', time, time, nil, query_data) }. + join + end + end + + describe '#subscribe_to_action_view' do + let(:subscription) { transaction.subscribe_to_action_view } + let(:time) { Time.now } + let(:view_data) { { identifier: 'foo.rb' } } + + after do + ActiveSupport::Notifications.unsubscribe(subscription) + end + + it 'tracks rendered views' do + expect(transaction).to receive(:track_view).with('foo.rb') + + subscription.publish('test', time, time, nil, view_data) + end + + it 'only tracks views rendered from the transaction thread' do + expect(transaction).to_not receive(:track_view) + + Thread.new { subscription.publish('test', time, time, nil, view_data) }. + join + end + end +end diff --git a/spec/lib/gitlab/uploads_transfer_spec.rb b/spec/lib/gitlab/uploads_transfer_spec.rb new file mode 100644 index 00000000000..260364a513e --- /dev/null +++ b/spec/lib/gitlab/uploads_transfer_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' + +describe Gitlab::UploadsTransfer do + before do + @root_dir = File.join(Rails.root, "public", "uploads") + @upload_transfer = Gitlab::UploadsTransfer.new + + @project_path_was = "test_project_was" + @project_path = "test_project" + @namespace_path_was = "test_namespace_was" + @namespace_path = "test_namespace" + end + + after do + FileUtils.rm_rf([ + File.join(@root_dir, @namespace_path), + File.join(@root_dir, @namespace_path_was) + ]) + end + + describe '#move_project' do + it "moves project upload to another namespace" do + FileUtils.mkdir_p(File.join(@root_dir, @namespace_path_was, @project_path)) + @upload_transfer.move_project(@project_path, @namespace_path_was, @namespace_path) + + expected_path = File.join(@root_dir, @namespace_path, @project_path) + expect(Dir.exist?(expected_path)).to be_truthy + end + end + + describe '#rename_project' do + it "renames project" do + FileUtils.mkdir_p(File.join(@root_dir, @namespace_path, @project_path_was)) + @upload_transfer.rename_project(@project_path_was, @project_path, @namespace_path) + + expected_path = File.join(@root_dir, @namespace_path, @project_path) + expect(Dir.exist?(expected_path)).to be_truthy + end + end + + describe '#rename_namespace' do + it "renames namespace" do + FileUtils.mkdir_p(File.join(@root_dir, @namespace_path_was, @project_path)) + @upload_transfer.rename_namespace(@namespace_path_was, @namespace_path) + + expected_path = File.join(@root_dir, @namespace_path, @project_path) + expect(Dir.exist?(expected_path)).to be_truthy + end + end +end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index cb67ec95d57..47863d54579 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -468,7 +468,7 @@ describe Notify do subject { Notify.note_commit_email(recipient.id, note.id) } it_behaves_like 'a note email' - it_behaves_like 'an answer to an existing thread', 'commits' + it_behaves_like 'an answer to an existing thread', 'commit' it 'has the correct subject' do is_expected.to have_subject /#{commit.title} \(#{commit.short_id}\)/ diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index de0b2ef4cda..f01fe8bd398 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -28,11 +28,11 @@ require 'spec_helper' describe ApplicationSetting, models: true do - it { expect(ApplicationSetting.create_from_defaults).to be_valid } + let(:setting) { ApplicationSetting.create_from_defaults } - context 'restricted signup domains' do - let(:setting) { ApplicationSetting.create_from_defaults } + it { expect(setting).to be_valid } + context 'restricted signup domains' do it 'set single domain' do setting.restricted_signup_domains_raw = 'example.com' expect(setting.restricted_signup_domains).to eq(['example.com']) @@ -53,4 +53,26 @@ describe ApplicationSetting, models: true do expect(setting.restricted_signup_domains).to eq(['example.com', '*.example.com']) end end + + context 'shared runners' do + let(:gl_project) { create(:empty_project) } + + before do + allow_any_instance_of(Project).to receive(:current_application_settings).and_return(setting) + end + + subject { gl_project.ensure_gitlab_ci_project.shared_runners_enabled } + + context 'enabled' do + before { setting.update_attributes(shared_runners_enabled: true) } + + it { is_expected.to be_truthy } + end + + context 'disabled' do + before { setting.update_attributes(shared_runners_enabled: false) } + + it { is_expected.to be_falsey } + end + end end diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index d875015b991..7f5abb83ac2 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -200,13 +200,34 @@ describe Ci::Build do context 'returns variables' do subject { build.variables } - let(:variables) do + let(:predefined_variables) do + [ + { key: :CI_BUILD_NAME, value: 'test', public: true }, + { key: :CI_BUILD_STAGE, value: 'stage', public: true }, + ] + end + + let(:yaml_variables) do [ { key: :DB_NAME, value: 'postgres', public: true } ] end - it { is_expected.to eq(variables) } + before { build.update_attributes(stage: 'stage') } + + it { is_expected.to eq(predefined_variables + yaml_variables) } + + context 'for tag' do + let(:tag_variable) do + [ + { key: :CI_BUILD_TAG, value: 'master', public: true } + ] + end + + before { build.update_attributes(tag: true) } + + it { is_expected.to eq(tag_variable + predefined_variables + yaml_variables) } + end context 'and secure variables' do let(:secure_variables) do @@ -219,7 +240,7 @@ describe Ci::Build do build.project.variables << Ci::Variable.new(key: 'SECRET_KEY', value: 'secret_value') end - it { is_expected.to eq(variables + secure_variables) } + it { is_expected.to eq(predefined_variables + yaml_variables + secure_variables) } context 'and trigger variables' do let(:trigger) { FactoryGirl.create :ci_trigger, project: project } @@ -229,12 +250,17 @@ describe Ci::Build do { key: :TRIGGER_KEY, value: 'TRIGGER_VALUE', public: false } ] end + let(:predefined_trigger_variable) do + [ + { key: :CI_BUILD_TRIGGERED, value: 'true', public: true } + ] + end before do build.trigger_request = trigger_request end - it { is_expected.to eq(variables + secure_variables + trigger_variables) } + it { is_expected.to eq(predefined_variables + predefined_trigger_variable + yaml_variables + secure_variables + trigger_variables) } end end end @@ -273,4 +299,105 @@ describe Ci::Build do is_expected.to eq(['rec1', pusher_email]) end end + + describe :can_be_served? do + let(:runner) { FactoryGirl.create :ci_specific_runner } + + before { build.project.runners << runner } + + context 'runner without tags' do + it 'can handle builds without tags' do + expect(build.can_be_served?(runner)).to be_truthy + end + + it 'cannot handle build with tags' do + build.tag_list = ['aa'] + expect(build.can_be_served?(runner)).to be_falsey + end + end + + context 'runner with tags' do + before { runner.tag_list = ['bb', 'cc'] } + + it 'can handle builds without tags' do + expect(build.can_be_served?(runner)).to be_truthy + end + + it 'can handle build with matching tags' do + build.tag_list = ['bb'] + expect(build.can_be_served?(runner)).to be_truthy + end + + it 'cannot handle build with not matching tags' do + build.tag_list = ['aa'] + expect(build.can_be_served?(runner)).to be_falsey + end + end + end + + describe :any_runners_online? do + subject { build.any_runners_online? } + + context 'when no runners' do + it { is_expected.to be_falsey } + end + + context 'if there are runner' do + let(:runner) { FactoryGirl.create :ci_specific_runner } + + before do + build.project.runners << runner + runner.update_attributes(contacted_at: 1.second.ago) + end + + it { is_expected.to be_truthy } + + it 'that is inactive' do + runner.update_attributes(active: false) + is_expected.to be_falsey + end + + it 'that is not online' do + runner.update_attributes(contacted_at: nil) + is_expected.to be_falsey + end + + it 'that cannot handle build' do + expect_any_instance_of(Ci::Build).to receive(:can_be_served?).and_return(false) + is_expected.to be_falsey + end + + end + end + + describe :show_warning? do + subject { build.show_warning? } + + %w(pending).each do |state| + context "if commit_status.status is #{state}" do + before { build.status = state } + + it { is_expected.to be_truthy } + + context "and there are specific runner" do + let(:runner) { FactoryGirl.create :ci_specific_runner, contacted_at: 1.second.ago } + + before do + build.project.runners << runner + runner.save + end + + it { is_expected.to be_falsey } + end + end + end + + %w(success failed canceled running).each do |state| + context "if commit_status.status is #{state}" do + before { build.status = state } + + it { is_expected.to be_falsey } + end + end + end end diff --git a/spec/models/ci/commit_spec.rb b/spec/models/ci/commit_spec.rb index 330971174fb..44dbd083f06 100644 --- a/spec/models/ci/commit_spec.rb +++ b/spec/models/ci/commit_spec.rb @@ -32,6 +32,24 @@ describe Ci::Commit do it { is_expected.to respond_to :git_author_email } it { is_expected.to respond_to :short_sha } + describe :ordered do + let(:project) { FactoryGirl.create :empty_project } + + it 'returns ordered list of commits' do + commit1 = FactoryGirl.create :ci_commit, committed_at: 1.hour.ago, gl_project: project + commit2 = FactoryGirl.create :ci_commit, committed_at: 2.hour.ago, gl_project: project + expect(project.ci_commits.ordered).to eq([commit2, commit1]) + end + + it 'returns commits ordered by committed_at and id, with nulls last' do + commit1 = FactoryGirl.create :ci_commit, committed_at: 1.hour.ago, gl_project: project + commit2 = FactoryGirl.create :ci_commit, committed_at: nil, gl_project: project + commit3 = FactoryGirl.create :ci_commit, committed_at: 2.hour.ago, gl_project: project + commit4 = FactoryGirl.create :ci_commit, committed_at: nil, gl_project: project + expect(project.ci_commits.ordered).to eq([commit2, commit4, commit3, commit1]) + end + end + describe :last_build do subject { commit.last_build } before do @@ -143,28 +161,28 @@ describe Ci::Commit do end describe :create_builds do - let(:commit) { FactoryGirl.create :ci_commit, gl_project: gl_project } + let!(:commit) { FactoryGirl.create :ci_commit, gl_project: gl_project } def create_builds(trigger_request = nil) commit.create_builds('master', false, nil, trigger_request) end - def create_next_builds(trigger_request = nil) - commit.create_next_builds('master', false, nil, trigger_request) + def create_next_builds + commit.create_next_builds(commit.builds.order(:id).last) end it 'creates builds' do expect(create_builds).to be_truthy - commit.builds.reload - expect(commit.builds.size).to eq(2) + commit.builds.update_all(status: "success") + expect(commit.builds.count(:all)).to eq(2) expect(create_next_builds).to be_truthy - commit.builds.reload - expect(commit.builds.size).to eq(4) + commit.builds.update_all(status: "success") + expect(commit.builds.count(:all)).to eq(4) expect(create_next_builds).to be_truthy - commit.builds.reload - expect(commit.builds.size).to eq(5) + commit.builds.update_all(status: "success") + expect(commit.builds.count(:all)).to eq(5) expect(create_next_builds).to be_falsey end @@ -176,12 +194,12 @@ describe Ci::Commit do it 'creates builds' do expect(create_builds).to be_truthy - commit.builds.reload - expect(commit.builds.size).to eq(2) + commit.builds.update_all(status: "success") + expect(commit.builds.count(:all)).to eq(2) expect(create_develop_builds).to be_truthy - commit.builds.reload - expect(commit.builds.size).to eq(4) + commit.builds.update_all(status: "success") + expect(commit.builds.count(:all)).to eq(4) expect(commit.refs.size).to eq(2) expect(commit.builds.pluck(:name).uniq.size).to eq(2) end @@ -193,28 +211,24 @@ describe Ci::Commit do it 'creates builds' do expect(create_builds(trigger_request)).to be_truthy - commit.builds.reload - expect(commit.builds.size).to eq(2) + expect(commit.builds.count(:all)).to eq(2) end it 'rebuilds commit' do expect(create_builds).to be_truthy - commit.builds.reload - expect(commit.builds.size).to eq(2) + expect(commit.builds.count(:all)).to eq(2) expect(create_builds(trigger_request)).to be_truthy - commit.builds.reload - expect(commit.builds.size).to eq(4) + expect(commit.builds.count(:all)).to eq(4) end it 'creates next builds' do expect(create_builds(trigger_request)).to be_truthy - commit.builds.reload - expect(commit.builds.size).to eq(2) + expect(commit.builds.count(:all)).to eq(2) + commit.builds.update_all(status: "success") - expect(create_next_builds(trigger_request)).to be_truthy - commit.builds.reload - expect(commit.builds.size).to eq(4) + expect(create_next_builds).to be_truthy + expect(commit.builds.count(:all)).to eq(4) end context 'for [ci skip]' do @@ -224,7 +238,7 @@ describe Ci::Commit do it 'rebuilds commit' do expect(commit.status).to eq('skipped') - expect(create_builds(trigger_request)).to be_truthy + expect(create_builds).to be_truthy # since everything in Ci::Commit is cached we need to fetch a new object new_commit = Ci::Commit.find_by_id(commit.id) @@ -232,6 +246,129 @@ describe Ci::Commit do end end end + + context 'properly creates builds when "when" is defined' do + let(:yaml) do + { + stages: ["build", "test", "test_failure", "deploy", "cleanup"], + build: { + stage: "build", + script: "BUILD", + }, + test: { + stage: "test", + script: "TEST", + }, + test_failure: { + stage: "test_failure", + script: "ON test failure", + when: "on_failure", + }, + deploy: { + stage: "deploy", + script: "PUBLISH", + }, + cleanup: { + stage: "cleanup", + script: "TIDY UP", + when: "always", + } + } + end + + before do + stub_ci_commit_yaml_file(YAML.dump(yaml)) + end + + it 'properly creates builds' do + expect(create_builds).to be_truthy + expect(commit.builds.pluck(:name)).to contain_exactly('build') + expect(commit.builds.pluck(:status)).to contain_exactly('pending') + commit.builds.running_or_pending.each(&:success) + + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending') + commit.builds.running_or_pending.each(&:success) + + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending') + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy') + commit.builds.running_or_pending.each(&:success) + + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'pending') + commit.builds.running_or_pending.each(&:success) + + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'success') + expect(commit.status).to eq('success') + end + + it 'properly creates builds when test fails' do + expect(create_builds).to be_truthy + expect(commit.builds.pluck(:name)).to contain_exactly('build') + expect(commit.builds.pluck(:status)).to contain_exactly('pending') + commit.builds.running_or_pending.each(&:success) + + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending') + commit.builds.running_or_pending.each(&:drop) + + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending') + commit.builds.running_or_pending.each(&:success) + + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'pending') + commit.builds.running_or_pending.each(&:success) + + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'success') + expect(commit.status).to eq('failed') + end + + it 'properly creates builds when test and test_failure fails' do + expect(create_builds).to be_truthy + expect(commit.builds.pluck(:name)).to contain_exactly('build') + expect(commit.builds.pluck(:status)).to contain_exactly('pending') + commit.builds.running_or_pending.each(&:success) + + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending') + commit.builds.running_or_pending.each(&:drop) + + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending') + commit.builds.running_or_pending.each(&:drop) + + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'pending') + commit.builds.running_or_pending.each(&:success) + + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'success') + expect(commit.status).to eq('failed') + end + + it 'properly creates builds when deploy fails' do + expect(create_builds).to be_truthy + expect(commit.builds.pluck(:name)).to contain_exactly('build') + expect(commit.builds.pluck(:status)).to contain_exactly('pending') + commit.builds.running_or_pending.each(&:success) + + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'pending') + commit.builds.running_or_pending.each(&:success) + + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending') + commit.builds.running_or_pending.each(&:drop) + + expect(commit.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup') + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'pending') + commit.builds.running_or_pending.each(&:success) + + expect(commit.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'success') + expect(commit.status).to eq('failed') + end + end end describe "#finished_at" do @@ -281,59 +418,4 @@ describe Ci::Commit do expect(commit.coverage).to be_nil end end - - describe :should_create_next_builds? do - before do - @build1 = FactoryGirl.create :ci_build, commit: commit, name: 'build1', ref: 'master', tag: false, status: 'success' - @build2 = FactoryGirl.create :ci_build, commit: commit, name: 'build1', ref: 'develop', tag: false, status: 'failed' - @build3 = FactoryGirl.create :ci_build, commit: commit, name: 'build1', ref: 'master', tag: true, status: 'failed' - @build4 = FactoryGirl.create :ci_build, commit: commit, name: 'build4', ref: 'master', tag: false, status: 'success' - end - - context 'for success' do - it 'to create if all succeeded' do - expect(commit.should_create_next_builds?(@build4)).to be_truthy - end - end - - context 'for failed' do - before do - @build4.update_attributes(status: 'failed') - end - - it 'to not create' do - expect(commit.should_create_next_builds?(@build4)).to be_falsey - end - - context 'and ignore failures for current' do - before do - @build4.update_attributes(allow_failure: true) - end - - it 'to create' do - expect(commit.should_create_next_builds?(@build4)).to be_truthy - end - end - end - - context 'for running' do - before do - @build4.update_attributes(status: 'running') - end - - it 'to not create' do - expect(commit.should_create_next_builds?(@build4)).to be_falsey - end - end - - context 'for retried' do - before do - @build5 = FactoryGirl.create :ci_build, commit: commit, name: 'build4', ref: 'master', tag: false, status: 'failed' - end - - it 'to not create' do - expect(commit.should_create_next_builds?(@build4)).to be_falsey - end - end - end end diff --git a/spec/models/ci/project_spec.rb b/spec/models/ci/project_spec.rb index dec4720a711..490c6a67982 100644 --- a/spec/models/ci/project_spec.rb +++ b/spec/models/ci/project_spec.rb @@ -131,24 +131,6 @@ describe Ci::Project do end end - describe 'ordered commits' do - let(:project) { FactoryGirl.create :empty_project } - - it 'returns ordered list of commits' do - commit1 = FactoryGirl.create :ci_commit, committed_at: 1.hour.ago, gl_project: project - commit2 = FactoryGirl.create :ci_commit, committed_at: 2.hour.ago, gl_project: project - expect(project.ci_commits).to eq([commit2, commit1]) - end - - it 'returns commits ordered by committed_at and id, with nulls last' do - commit1 = FactoryGirl.create :ci_commit, committed_at: 1.hour.ago, gl_project: project - commit2 = FactoryGirl.create :ci_commit, committed_at: nil, gl_project: project - commit3 = FactoryGirl.create :ci_commit, committed_at: 2.hour.ago, gl_project: project - commit4 = FactoryGirl.create :ci_commit, committed_at: nil, gl_project: project - expect(project.ci_commits).to eq([commit2, commit4, commit3, commit1]) - end - end - context :valid_project do let(:commit) { FactoryGirl.create(:ci_commit) } @@ -259,5 +241,18 @@ describe Ci::Project do FactoryGirl.create(:ci_shared_runner) expect(project.any_runners?).to be_falsey end + + it "checks the presence of specific runner" do + project = FactoryGirl.create(:ci_project) + specific_runner = FactoryGirl.create(:ci_specific_runner) + project.runners << specific_runner + expect(project.any_runners? { |runner| runner == specific_runner }).to be_truthy + end + + it "checks the presence of shared runner" do + project = FactoryGirl.create(:ci_project, shared_runners_enabled: true) + shared_runner = FactoryGirl.create(:ci_shared_runner) + expect(project.any_runners? { |runner| runner == shared_runner }).to be_truthy + end end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 757593a7ab8..f8a51c29dc2 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -32,7 +32,7 @@ describe Ci::Runner do end it 'should return the token if the description is an empty string' do - runner = FactoryGirl.build(:ci_runner, description: '') + runner = FactoryGirl.build(:ci_runner, description: '', token: 'token') expect(runner.display_name).to eq runner.token end end @@ -48,6 +48,71 @@ describe Ci::Runner do it { expect(shared_runner.only_for?(project)).to be_truthy } end + describe :online do + subject { Ci::Runner.online } + + before do + @runner1 = FactoryGirl.create(:ci_shared_runner, contacted_at: 1.year.ago) + @runner2 = FactoryGirl.create(:ci_shared_runner, contacted_at: 1.second.ago) + end + + it { is_expected.to eq([@runner2])} + end + + describe :online? do + let(:runner) { FactoryGirl.create(:ci_shared_runner) } + + subject { runner.online? } + + context 'never contacted' do + before { runner.contacted_at = nil } + + it { is_expected.to be_falsey } + end + + context 'contacted long time ago time' do + before { runner.contacted_at = 1.year.ago } + + it { is_expected.to be_falsey } + end + + context 'contacted 1s ago' do + before { runner.contacted_at = 1.second.ago } + + it { is_expected.to be_truthy } + end + end + + describe :status do + let(:runner) { FactoryGirl.create(:ci_shared_runner, contacted_at: 1.second.ago) } + + subject { runner.status } + + context 'never connected' do + before { runner.contacted_at = nil } + + it { is_expected.to eq(:not_connected) } + end + + context 'contacted 1s ago' do + before { runner.contacted_at = 1.second.ago } + + it { is_expected.to eq(:online) } + end + + context 'contacted long time ago' do + before { runner.contacted_at = 1.year.ago } + + it { is_expected.to eq(:offline) } + end + + context 'inactive' do + before { runner.active = false } + + it { is_expected.to eq(:paused) } + end + end + describe "belongs_to_one_project?" do it "returns false if there are two projects runner assigned to" do runner = FactoryGirl.create(:ci_specific_runner) diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index e303a97e6b5..90be9324951 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -89,9 +89,9 @@ eos end it_behaves_like 'a mentionable' do - subject { commit } + subject { create(:project).commit } - let(:author) { create(:user, email: commit.author_email) } + let(:author) { create(:user, email: subject.author_email) } let(:backref_text) { "commit #{subject.id}" } let(:set_mentionable_text) do ->(txt) { allow(subject).to receive(:safe_message).and_return(txt) } diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 8f706f8934b..0f13c4410cd 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -68,7 +68,6 @@ describe Issue, "Issuable" do end end - describe "#to_hook_data" do let(:hook_data) { issue.to_hook_data(user) } diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 2d6fe003215..6179882e935 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -25,7 +25,7 @@ describe Issue, "Mentionable" do it 'correctly removes already-mentioned Commits' do expect(SystemNoteService).not_to receive(:cross_reference) - issue.create_cross_references!(project, author, [commit2]) + issue.create_cross_references!(author, [commit2]) end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 80638fc8db2..0f23e81ace9 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -84,4 +84,23 @@ describe Group do expect(group.avatar_type).to eq(["only images allowed"]) end end + + describe "public_profile?" do + it "returns true for public group" do + group = create(:group, public: true) + expect(group.public_profile?).to be_truthy + end + + it "returns true for non-public group with public project" do + group = create(:group) + create(:project, :public, group: group) + expect(group.public_profile?).to be_truthy + end + + it "returns false for non-public group with no public projects" do + group = create(:group) + create(:project, group: group) + expect(group.public_profile?).to be_falsy + end + end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index cf336d82957..c9aa1b063c6 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -68,8 +68,45 @@ describe Issue do end end + describe '#closed_by_merge_requests' do + let(:project) { create(:project) } + let(:issue) { create(:issue, project: project, state: "opened")} + let(:closed_issue) { build(:issue, project: project, state: "closed")} + + let(:mr) do + opts = { + title: 'Awesome merge_request', + description: "Fixes #{issue.to_reference}", + source_branch: 'feature', + target_branch: 'master' + } + MergeRequests::CreateService.new(project, project.owner, opts).execute + end + + let(:closed_mr) do + opts = { + title: 'Awesome merge_request 2', + description: "Fixes #{issue.to_reference}", + source_branch: 'feature', + target_branch: 'master', + state: 'closed' + } + MergeRequests::CreateService.new(project, project.owner, opts).execute + end + + it 'returns the merge request to close this issue' do + allow(mr).to receive(:closes_issue?).with(issue).and_return(true) + + expect(issue.closed_by_merge_requests).to eq([mr]) + end + + it "returns an empty array when the current issue is closed already" do + expect(closed_issue.closed_by_merge_requests).to eq([]) + end + end + it_behaves_like 'an editable mentionable' do - subject { create(:issue, project: project) } + subject { create(:issue) } let(:backref_text) { "issue #{subject.to_reference}" } let(:set_mentionable_text) { ->(txt){ subject.description = txt } } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 6aaf1c036b0..eed2cbc5412 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -79,6 +79,12 @@ describe MergeRequest do expect(merge_request.commits).not_to be_empty expect(merge_request.mr_and_commit_notes.count).to eq(2) end + + it "should include notes for commits from target project as well" do + create(:note, commit_id: merge_request.commits.first.id, noteable_type: 'Commit', project: merge_request.target_project) + expect(merge_request.commits).not_to be_empty + expect(merge_request.mr_and_commit_notes.count).to eq(3) + end end describe '#is_being_reassigned?' do diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index c88d5349663..77c58627322 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -140,4 +140,32 @@ describe Milestone do end end + describe '#sort_issues' do + let(:milestone) { create(:milestone) } + + let(:issue1) { create(:issue, milestone: milestone, position: 1) } + let(:issue2) { create(:issue, milestone: milestone, position: 2) } + let(:issue3) { create(:issue, milestone: milestone, position: 3) } + let(:issue4) { create(:issue, position: 42) } + + it 'sorts the given issues' do + milestone.sort_issues([issue3.id, issue2.id, issue1.id]) + + issue1.reload + issue2.reload + issue3.reload + + expect(issue1.position).to eq(3) + expect(issue2.position).to eq(2) + expect(issue3.position).to eq(1) + end + + it 'ignores issues not part of the milestone' do + milestone.sort_issues([issue3.id, issue2.id, issue1.id, issue4.id]) + + issue4.reload + + expect(issue4.position).to eq(42) + end + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 3a0b194ba1e..75564839dcf 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -192,10 +192,9 @@ describe Note do end it_behaves_like 'an editable mentionable' do - subject { create :note, noteable: issue, project: project } + subject { create :note, noteable: issue, project: issue.project } - let(:project) { create(:project) } - let(:issue) { create :issue, project: project } + let(:issue) { create :issue } let(:backref_text) { issue.gfm_reference } let(:set_mentionable_text) { ->(txt) { subject.note = txt } } end diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb index f8a3493f52d..c34b2487ecf 100644 --- a/spec/models/project_services/bamboo_service_spec.rb +++ b/spec/models/project_services/bamboo_service_spec.rb @@ -30,27 +30,65 @@ describe BambooService, models: true do let(:user) { create(:user) } let(:project) { create(:project) } - before do - @bamboo_service = BambooService.create( - project: create(:project), - properties: { - bamboo_url: 'http://gitlab.com', - username: 'mic', - password: "password" - } - ) - end + context "when a password was previously set" do + before do + @bamboo_service = BambooService.create( + project: create(:project), + properties: { + bamboo_url: 'http://gitlab.com', + username: 'mic', + password: "password" + } + ) + end + + it "reset password if url changed" do + @bamboo_service.bamboo_url = 'http://gitlab1.com' + @bamboo_service.save + expect(@bamboo_service.password).to be_nil + end + + it "does not reset password if username changed" do + @bamboo_service.username = "some_name" + @bamboo_service.save + expect(@bamboo_service.password).to eq("password") + end + + it "does not reset password if new url is set together with password, even if it's the same password" do + @bamboo_service.bamboo_url = 'http://gitlab_edited.com' + @bamboo_service.password = 'password' + @bamboo_service.save + expect(@bamboo_service.password).to eq("password") + expect(@bamboo_service.bamboo_url).to eq("http://gitlab_edited.com") + end - it "reset password if url changed" do - @bamboo_service.bamboo_url = 'http://gitlab1.com' - @bamboo_service.save - expect(@bamboo_service.password).to be_nil + it "should reset password if url changed, even if setter called multiple times" do + @bamboo_service.bamboo_url = 'http://gitlab1.com' + @bamboo_service.bamboo_url = 'http://gitlab1.com' + @bamboo_service.save + expect(@bamboo_service.password).to be_nil + end end + + context "when no password was previously set" do + before do + @bamboo_service = BambooService.create( + project: create(:project), + properties: { + bamboo_url: 'http://gitlab.com', + username: 'mic' + } + ) + end + + it "saves password if new url is set together with password" do + @bamboo_service.bamboo_url = 'http://gitlab_edited.com' + @bamboo_service.password = 'password' + @bamboo_service.save + expect(@bamboo_service.password).to eq("password") + expect(@bamboo_service.bamboo_url).to eq("http://gitlab_edited.com") + end - it "does not reset password if username changed" do - @bamboo_service.username = "some_name" - @bamboo_service.save - expect(@bamboo_service.password).to eq("password") end end end diff --git a/spec/models/project_services/gitlab_ci_service_spec.rb b/spec/models/project_services/gitlab_ci_service_spec.rb index 842089ebe0d..b9006b693b2 100644 --- a/spec/models/project_services/gitlab_ci_service_spec.rb +++ b/spec/models/project_services/gitlab_ci_service_spec.rb @@ -39,7 +39,7 @@ describe GitlabCiService do end describe :build_page do - it { expect(@service.build_page("2ab7834c", 'master')).to eq("http://localhost/#{@ci_project.gl_project.path_with_namespace}/commit/2ab7834c/ci")} + it { expect(@service.build_page("2ab7834c", 'master')).to eq("http://localhost/#{@ci_project.gl_project.path_with_namespace}/commit/2ab7834c/builds")} end describe "execute" do diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb index 3dbd2346bcc..f26b47a856c 100644 --- a/spec/models/project_services/teamcity_service_spec.rb +++ b/spec/models/project_services/teamcity_service_spec.rb @@ -30,27 +30,64 @@ describe TeamcityService, models: true do let(:user) { create(:user) } let(:project) { create(:project) } - before do - @teamcity_service = TeamcityService.create( - project: create(:project), - properties: { - teamcity_url: 'http://gitlab.com', - username: 'mic', - password: "password" - } - ) - end + context "when a password was previously set" do + before do + @teamcity_service = TeamcityService.create( + project: create(:project), + properties: { + teamcity_url: 'http://gitlab.com', + username: 'mic', + password: "password" + } + ) + end + + it "reset password if url changed" do + @teamcity_service.teamcity_url = 'http://gitlab1.com' + @teamcity_service.save + expect(@teamcity_service.password).to be_nil + end + + it "does not reset password if username changed" do + @teamcity_service.username = "some_name" + @teamcity_service.save + expect(@teamcity_service.password).to eq("password") + end + + it "does not reset password if new url is set together with password, even if it's the same password" do + @teamcity_service.teamcity_url = 'http://gitlab_edited.com' + @teamcity_service.password = 'password' + @teamcity_service.save + expect(@teamcity_service.password).to eq("password") + expect(@teamcity_service.teamcity_url).to eq("http://gitlab_edited.com") + end - it "reset password if url changed" do - @teamcity_service.teamcity_url = 'http://gitlab1.com' - @teamcity_service.save - expect(@teamcity_service.password).to be_nil + it "should reset password if url changed, even if setter called multiple times" do + @teamcity_service.teamcity_url = 'http://gitlab1.com' + @teamcity_service.teamcity_url = 'http://gitlab1.com' + @teamcity_service.save + expect(@teamcity_service.password).to be_nil + end end + + context "when no password was previously set" do + before do + @teamcity_service = TeamcityService.create( + project: create(:project), + properties: { + teamcity_url: 'http://gitlab.com', + username: 'mic' + } + ) + end - it "does not reset password if username changed" do - @teamcity_service.username = "some_name" - @teamcity_service.save - expect(@teamcity_service.password).to eq("password") + it "saves password if new url is set together with password" do + @teamcity_service.teamcity_url = 'http://gitlab_edited.com' + @teamcity_service.password = 'password' + @teamcity_service.save + expect(@teamcity_service.password).to eq("password") + expect(@teamcity_service.teamcity_url).to eq("http://gitlab_edited.com") + end end end end diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 94802dcfb79..9f6cdeeaa96 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -223,7 +223,7 @@ describe ProjectWiki do def create_temp_repo(path) FileUtils.mkdir_p path - system(*%W(git init --quiet --bare -- #{path})) + system(*%W(#{Gitlab.config.git.bin_path} init --quiet --bare -- #{path})) end def remove_temp_repo(path) diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb new file mode 100644 index 00000000000..527005b2b69 --- /dev/null +++ b/spec/models/release_spec.rb @@ -0,0 +1,16 @@ +require 'rails_helper' + +RSpec.describe Release, type: :model do + let(:release) { create(:release) } + + it { expect(release).to be_valid } + + describe 'associations' do + it { is_expected.to belong_to(:project) } + end + + describe 'validation' do + it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_presence_of(:description) } + end +end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 60b93988476..04437d64fee 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -26,6 +26,15 @@ describe Repository do it { is_expected.to eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8') } end + describe :find_commits_by_message do + subject { repository.find_commits_by_message('submodule').map{ |k| k.id } } + + it { is_expected.to include('5937ac0a7beb003549fc5fd26fc247adbce4a52e') } + it { is_expected.to include('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } + it { is_expected.to include('cfe32cf61b73a0d5e9f13e774abde7ff789b1660') } + it { is_expected.not_to include('913c66a37b4a45b9769037c55c2d238bd0942d2e') } + end + describe :blob_at do context 'blank sha' do subject { repository.blob_at(Gitlab::Git::BLANK_SHA, '.gitignore') } diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index da87ea5b84f..692e5fda3ba 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -104,7 +104,7 @@ describe Service do end end - describe "#prop_updated?" do + describe "{property}_changed?" do let(:service) do BambooService.create( project: create(:project), @@ -116,14 +116,112 @@ describe Service do ) end - it "returns false" do + it "returns false when the property has not been assigned a new value" do service.username = "key_changed" - expect(service.prop_updated?(:bamboo_url)).to be_falsy + expect(service.bamboo_url_changed?).to be_falsy end - it "returns true" do - service.bamboo_url = "http://other.com" - expect(service.prop_updated?(:bamboo_url)).to be_truthy + it "returns true when the property has been assigned a different value" do + service.bamboo_url = "http://example.com" + expect(service.bamboo_url_changed?).to be_truthy + end + + it "returns true when the property has been assigned a different value twice" do + service.bamboo_url = "http://example.com" + service.bamboo_url = "http://example.com" + expect(service.bamboo_url_changed?).to be_truthy + end + + it "returns false when the property has been re-assigned the same value" do + service.bamboo_url = 'http://gitlab.com' + expect(service.bamboo_url_changed?).to be_falsy + end + + it "returns false when the property has been assigned a new value then saved" do + service.bamboo_url = 'http://example.com' + service.save + expect(service.bamboo_url_changed?).to be_falsy + end + end + + describe "{property}_touched?" do + let(:service) do + BambooService.create( + project: create(:project), + properties: { + bamboo_url: 'http://gitlab.com', + username: 'mic', + password: "password" + } + ) + end + + it "returns false when the property has not been assigned a new value" do + service.username = "key_changed" + expect(service.bamboo_url_touched?).to be_falsy + end + + it "returns true when the property has been assigned a different value" do + service.bamboo_url = "http://example.com" + expect(service.bamboo_url_touched?).to be_truthy + end + + it "returns true when the property has been assigned a different value twice" do + service.bamboo_url = "http://example.com" + service.bamboo_url = "http://example.com" + expect(service.bamboo_url_touched?).to be_truthy + end + + it "returns true when the property has been re-assigned the same value" do + service.bamboo_url = 'http://gitlab.com' + expect(service.bamboo_url_touched?).to be_truthy + end + + it "returns false when the property has been assigned a new value then saved" do + service.bamboo_url = 'http://example.com' + service.save + expect(service.bamboo_url_changed?).to be_falsy + end + end + + describe "{property}_was" do + let(:service) do + BambooService.create( + project: create(:project), + properties: { + bamboo_url: 'http://gitlab.com', + username: 'mic', + password: "password" + } + ) + end + + + it "returns nil when the property has not been assigned a new value" do + service.username = "key_changed" + expect(service.bamboo_url_was).to be_nil + end + + it "returns the previous value when the property has been assigned a different value" do + service.bamboo_url = "http://example.com" + expect(service.bamboo_url_was).to eq('http://gitlab.com') + end + + it "returns initial value when the property has been re-assigned the same value" do + service.bamboo_url = 'http://gitlab.com' + expect(service.bamboo_url_was).to eq('http://gitlab.com') + end + + it "returns initial value when the property has been assigned multiple values" do + service.bamboo_url = "http://example.com" + service.bamboo_url = "http://example2.com" + expect(service.bamboo_url_was).to eq('http://gitlab.com') + end + + it "returns nil when the property has been assigned a new value then saved" do + service.bamboo_url = 'http://example.com' + service.save + expect(service.bamboo_url_was).to be_nil end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c71cfb3ebe3..49e0bfdd2ec 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -663,24 +663,24 @@ describe User do @user1 = create :user, created_at: Date.today - 1, last_sign_in_at: Date.today - 1, name: 'Omega' end - it "sorts users as recently_signed_in" do + it "sorts users by the recent sign-in time" do expect(User.sort('recent_sign_in').first).to eq(@user) end - it "sorts users as late_signed_in" do + it "sorts users by the oldest sign-in time" do expect(User.sort('oldest_sign_in').first).to eq(@user1) end - it "sorts users as recently_created" do + it "sorts users in descending order by their creation time" do expect(User.sort('created_desc').first).to eq(@user) end - it "sorts users as late_created" do + it "sorts users in ascending order by their creation time" do expect(User.sort('created_asc').first).to eq(@user1) end - it "sorts users by name when nil is passed" do - expect(User.sort(nil).first).to eq(@user) + it "sorts users by id in descending order when nil is passed" do + expect(User.sort(nil).first).to eq(@user1) end end diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb index 4048c297013..0c19094ec54 100644 --- a/spec/requests/api/api_helpers_spec.rb +++ b/spec/requests/api/api_helpers_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe API, api: true do - include API::APIHelpers + include API::Helpers include ApiHelpers let(:user) { create(:user) } let(:admin) { create(:admin) } @@ -13,25 +13,25 @@ describe API, api: true do def set_env(token_usr, identifier) clear_env clear_param - env[API::APIHelpers::PRIVATE_TOKEN_HEADER] = token_usr.private_token - env[API::APIHelpers::SUDO_HEADER] = identifier + env[API::Helpers::PRIVATE_TOKEN_HEADER] = token_usr.private_token + env[API::Helpers::SUDO_HEADER] = identifier end def set_param(token_usr, identifier) clear_env clear_param - params[API::APIHelpers::PRIVATE_TOKEN_PARAM] = token_usr.private_token - params[API::APIHelpers::SUDO_PARAM] = identifier + params[API::Helpers::PRIVATE_TOKEN_PARAM] = token_usr.private_token + params[API::Helpers::SUDO_PARAM] = identifier end def clear_env - env.delete(API::APIHelpers::PRIVATE_TOKEN_HEADER) - env.delete(API::APIHelpers::SUDO_HEADER) + env.delete(API::Helpers::PRIVATE_TOKEN_HEADER) + env.delete(API::Helpers::SUDO_HEADER) end def clear_param - params.delete(API::APIHelpers::PRIVATE_TOKEN_PARAM) - params.delete(API::APIHelpers::SUDO_PARAM) + params.delete(API::Helpers::PRIVATE_TOKEN_PARAM) + params.delete(API::Helpers::SUDO_PARAM) end def error!(message, status) @@ -40,22 +40,22 @@ describe API, api: true do describe ".current_user" do it "should return nil for an invalid token" do - env[API::APIHelpers::PRIVATE_TOKEN_HEADER] = 'invalid token' + env[API::Helpers::PRIVATE_TOKEN_HEADER] = 'invalid token' allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ false } expect(current_user).to be_nil end it "should return nil for a user without access" do - env[API::APIHelpers::PRIVATE_TOKEN_HEADER] = user.private_token + env[API::Helpers::PRIVATE_TOKEN_HEADER] = user.private_token allow(Gitlab::UserAccess).to receive(:allowed?).and_return(false) expect(current_user).to be_nil end it "should leave user as is when sudo not specified" do - env[API::APIHelpers::PRIVATE_TOKEN_HEADER] = user.private_token + env[API::Helpers::PRIVATE_TOKEN_HEADER] = user.private_token expect(current_user).to eq(user) clear_env - params[API::APIHelpers::PRIVATE_TOKEN_PARAM] = user.private_token + params[API::Helpers::PRIVATE_TOKEN_PARAM] = user.private_token expect(current_user).to eq(user) end diff --git a/spec/requests/api/commit_status_spec.rb b/spec/requests/api/commit_status_spec.rb index b9e6dfc15a7..a28607bd240 100644 --- a/spec/requests/api/commit_status_spec.rb +++ b/spec/requests/api/commit_status_spec.rb @@ -18,7 +18,7 @@ describe API::API, api: true do before do @status1 = create(:commit_status, commit: ci_commit, status: 'running') @status2 = create(:commit_status, commit: ci_commit, name: 'coverage', status: 'pending') - @status3 = create(:commit_status, commit: ci_commit, name: 'coverage', ref: 'develop', status: 'running') + @status3 = create(:commit_status, commit: ci_commit, name: 'coverage', ref: 'develop', status: 'running', allow_failure: true) @status4 = create(:commit_status, commit: ci_commit, name: 'coverage', status: 'success') @status5 = create(:commit_status, commit: ci_commit, ref: 'develop', status: 'success') @status6 = create(:commit_status, commit: ci_commit, status: 'success') @@ -30,6 +30,8 @@ describe API::API, api: true do expect(json_response).to be_an Array expect(statuses_id).to contain_exactly(@status3.id, @status4.id, @status5.id, @status6.id) + json_response.sort_by!{ |status| status['id'] } + expect(json_response.map{ |status| status['allow_failure'] }).to eq([true, false, false, false]) end it "should return all commit statuses" do diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 042e6352567..8efa09f75fd 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -19,6 +19,7 @@ describe API::API, api: true do expect(response.status).to eq(200) expect(json_response['file_path']).to eq(file_path) expect(json_response['file_name']).to eq('popen.rb') + expect(json_response['last_commit_id']).to eq('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') expect(Base64.decode64(json_response['content']).lines.first).to eq("require 'fileutils'\n") end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 35b3d3e296a..a68c7b1e461 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -379,9 +379,14 @@ describe API::API, api: true do describe "POST /projects/:id/merge_request/:merge_request_id/comments" do it "should return comment" do + original_count = merge_request.notes.size + post api("/projects/#{project.id}/merge_request/#{merge_request.id}/comments", user), note: "My comment" expect(response.status).to eq(201) expect(json_response['note']).to eq('My comment') + expect(json_response['author']['name']).to eq(user.name) + expect(json_response['author']['username']).to eq(user.username) + expect(merge_request.notes.size).to eq(original_count + 1) end it "should return 400 if note is missing" do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 580bbec77d1..e9de9e0826d 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -606,28 +606,42 @@ describe API::API, api: true do describe 'DELETE /projects/:id/fork' do - it "shouldn't available for non admin users" do + it "shouldn't be visible to users outside group" do delete api("/projects/#{project_fork_target.id}/fork", user) - expect(response.status).to eq(403) + expect(response.status).to eq(404) end - it 'should make forked project unforked' do - post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", admin) - project_fork_target.reload - expect(project_fork_target.forked_from_project).not_to be_nil - expect(project_fork_target.forked?).to be_truthy - delete api("/projects/#{project_fork_target.id}/fork", admin) - expect(response.status).to eq(200) - project_fork_target.reload - expect(project_fork_target.forked_from_project).to be_nil - expect(project_fork_target.forked?).not_to be_truthy - end + context 'when users belong to project group' do + let(:project_fork_target) { create(:project, group: create(:group)) } - it 'should be idempotent if not forked' do - expect(project_fork_target.forked_from_project).to be_nil - delete api("/projects/#{project_fork_target.id}/fork", admin) - expect(response.status).to eq(200) - expect(project_fork_target.reload.forked_from_project).to be_nil + before do + project_fork_target.group.add_owner user + project_fork_target.group.add_developer user2 + end + + it 'should be forbidden to non-owner users' do + delete api("/projects/#{project_fork_target.id}/fork", user2) + expect(response.status).to eq(403) + end + + it 'should make forked project unforked' do + post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", admin) + project_fork_target.reload + expect(project_fork_target.forked_from_project).not_to be_nil + expect(project_fork_target.forked?).to be_truthy + delete api("/projects/#{project_fork_target.id}/fork", admin) + expect(response.status).to eq(200) + project_fork_target.reload + expect(project_fork_target.forked_from_project).to be_nil + expect(project_fork_target.forked?).not_to be_truthy + end + + it 'should be idempotent if not forked' do + expect(project_fork_target.forked_from_project).to be_nil + delete api("/projects/#{project_fork_target.id}/fork", admin) + expect(response.status).to eq(200) + expect(project_fork_target.reload.forked_from_project).to be_nil + end end end end diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 09a79553f72..faf6b77a462 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -36,8 +36,8 @@ describe API::API, api: true do it 'should create a new annotated tag' do # Identity must be set in .gitconfig to create annotated tag. repo_path = project.repository.path_to_repo - system(*%W(git --git-dir=#{repo_path} config user.name #{user.name})) - system(*%W(git --git-dir=#{repo_path} config user.email #{user.email})) + system(*%W(#{Gitlab.config.git.bin_path} --git-dir=#{repo_path} config user.name #{user.name})) + system(*%W(#{Gitlab.config.git.bin_path} --git-dir=#{repo_path} config user.email #{user.email})) post api("/projects/#{project.id}/repository/tags", user), tag_name: 'v7.1.0', @@ -166,24 +166,21 @@ describe API::API, api: true do get api("/projects/#{project.id}/repository/archive", user) repo_name = project.repository.name.gsub("\.git", "") expect(response.status).to eq(200) - expect(response.headers['Content-Disposition']).to match(/filename\=\"#{repo_name}\-[^\.]+\.tar.gz\"/) - expect(response.content_type).to eq(MIME::Types.type_for('file.tar.gz').first.content_type) + expect(json_response['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.tar.gz/) end it "should get the archive.zip" do get api("/projects/#{project.id}/repository/archive.zip", user) repo_name = project.repository.name.gsub("\.git", "") expect(response.status).to eq(200) - expect(response.headers['Content-Disposition']).to match(/filename\=\"#{repo_name}\-[^\.]+\.zip\"/) - expect(response.content_type).to eq(MIME::Types.type_for('file.zip').first.content_type) + expect(json_response['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.zip/) end it "should get the archive.tar.bz2" do get api("/projects/#{project.id}/repository/archive.tar.bz2", user) repo_name = project.repository.name.gsub("\.git", "") expect(response.status).to eq(200) - expect(response.headers['Content-Disposition']).to match(/filename\=\"#{repo_name}\-[^\.]+\.tar.bz2\"/) - expect(response.content_type).to eq(MIME::Types.type_for('file.tar.bz2').first.content_type) + expect(json_response['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.tar.bz2/) end it "should return 404 for invalid sha" do diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index 54c1d0199f6..88218a93e1f 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -76,6 +76,8 @@ describe Ci::API::API do expect(response.status).to eq(201) expect(json_response["variables"]).to eq([ + { "key" => "CI_BUILD_NAME", "value" => "spinach", "public" => true }, + { "key" => "CI_BUILD_STAGE", "value" => "test", "public" => true }, { "key" => "DB_NAME", "value" => "postgres", "public" => true }, { "key" => "SECRET_KEY", "value" => "secret_value", "public" => false }, ]) @@ -93,6 +95,9 @@ describe Ci::API::API do expect(response.status).to eq(201) expect(json_response["variables"]).to eq([ + { "key" => "CI_BUILD_NAME", "value" => "spinach", "public" => true }, + { "key" => "CI_BUILD_STAGE", "value" => "test", "public" => true }, + { "key" => "CI_BUILD_TRIGGERED", "value" => "true", "public" => true }, { "key" => "DB_NAME", "value" => "postgres", "public" => true }, { "key" => "SECRET_KEY", "value" => "secret_value", "public" => false }, { "key" => "TRIGGER_KEY", "value" => "TRIGGER_VALUE", "public" => false }, diff --git a/spec/services/archive_repository_service_spec.rb b/spec/services/archive_repository_service_spec.rb index 0ec70c51b3a..f7a36cd9670 100644 --- a/spec/services/archive_repository_service_spec.rb +++ b/spec/services/archive_repository_service_spec.rb @@ -6,14 +6,14 @@ describe ArchiveRepositoryService do describe "#execute" do it "cleans old archives" do - expect(project.repository).to receive(:clean_old_archives) + expect(RepositoryArchiveCacheWorker).to receive(:perform_async) subject.execute(timeout: 0.0) end context "when the repository doesn't have an archive file path" do before do - allow(project.repository).to receive(:archive_file_path).and_return(nil) + allow(project.repository).to receive(:archive_metadata).and_return(Hash.new) end it "raises an error" do @@ -21,70 +21,5 @@ describe ArchiveRepositoryService do end end - context "when the repository has an archive file path" do - let(:file_path) { "/archive.zip" } - let(:pid_file_path) { "/archive.zip.pid" } - - before do - allow(project.repository).to receive(:archive_file_path).and_return(file_path) - allow(project.repository).to receive(:archive_pid_file_path).and_return(pid_file_path) - end - - context "when the archive file already exists" do - before do - allow(File).to receive(:exist?).with(file_path).and_return(true) - end - - it "returns the file path" do - expect(subject.execute(timeout: 0.0)).to eq(file_path) - end - end - - context "when the archive file doesn't exist yet" do - before do - allow(File).to receive(:exist?).with(file_path).and_return(false) - allow(File).to receive(:exist?).with(pid_file_path).and_return(true) - end - - context "when the archive pid file doesn't exist yet" do - before do - allow(File).to receive(:exist?).with(pid_file_path).and_return(false) - end - - it "queues the RepositoryArchiveWorker" do - expect(RepositoryArchiveWorker).to receive(:perform_async) - - subject.execute(timeout: 0.0) - end - end - - context "when the archive pid file already exists" do - it "doesn't queue the RepositoryArchiveWorker" do - expect(RepositoryArchiveWorker).not_to receive(:perform_async) - - subject.execute(timeout: 0.0) - end - end - - context "when the archive file exists after a little while" do - before do - Thread.new do - sleep 0.1 - allow(File).to receive(:exist?).with(file_path).and_return(true) - end - end - - it "returns the file path" do - expect(subject.execute(timeout: 0.2)).to eq(file_path) - end - end - - context "when the archive file doesn't exist after the timeout" do - it "returns nil" do - expect(subject.execute(timeout: 0.0)).to eq(nil) - end - end - end - end end end diff --git a/spec/services/ci/image_for_build_service_spec.rb b/spec/services/ci/image_for_build_service_spec.rb index d7242d684c6..cda7d0c4a51 100644 --- a/spec/services/ci/image_for_build_service_spec.rb +++ b/spec/services/ci/image_for_build_service_spec.rb @@ -4,8 +4,9 @@ module Ci describe ImageForBuildService do let(:service) { ImageForBuildService.new } let(:project) { FactoryGirl.create(:ci_project) } - let(:gl_project) { FactoryGirl.create(:empty_project, gitlab_ci_project: project) } - let(:commit) { FactoryGirl.create(:ci_commit, gl_project: gl_project, ref: 'master') } + let(:gl_project) { FactoryGirl.create(:project, gitlab_ci_project: project) } + let(:commit_sha) { gl_project.commit('master').sha } + let(:commit) { gl_project.ensure_ci_commit(commit_sha) } let(:build) { FactoryGirl.create(:ci_build, commit: commit) } describe :execute do diff --git a/spec/services/ci/register_build_service_spec.rb b/spec/services/ci/register_build_service_spec.rb index 781764627ac..b370dfbe113 100644 --- a/spec/services/ci/register_build_service_spec.rb +++ b/spec/services/ci/register_build_service_spec.rb @@ -70,6 +70,10 @@ module Ci end context 'disallow shared runners' do + before do + gl_project.gitlab_ci_project.update(shared_runners_enabled: false) + end + context 'shared runner' do let(:build) { service.execute(shared_runner) } diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index c483060fd73..17015d29e51 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -112,6 +112,14 @@ describe GitPushService do it { expect(@event.project).to eq(project) } it { expect(@event.action).to eq(Event::PUSHED) } it { expect(@event.data).to eq(service.push_data) } + + context "Updates merge requests" do + it "when pushing a new branch for the first time" do + expect(project).to receive(:update_merge_requests). + with(@blankrev, 'newrev', 'refs/heads/master', user) + service.execute(project, user, @blankrev, 'newrev', 'refs/heads/master') + end + end end describe "Web Hooks" do @@ -155,7 +163,7 @@ describe GitPushService do before do allow(commit).to receive_messages( - safe_message: "this commit \n mentions ##{issue.id}", + safe_message: "this commit \n mentions #{issue.to_reference}", references: [issue], author_name: commit_author.name, author_email: commit_author.email diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 9516e7936d8..7ee4488521d 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -62,6 +62,25 @@ describe MergeRequests::RefreshService do it { expect(@fork_merge_request.notes.last.note).to include('changed to merged') } end + context 'manual merge of source branch' do + before do + # Merge master -> feature branch + author = { email: 'test@gitlab.com', time: Time.now, name: "Me" } + commit_options = { message: 'Test message', committer: author, author: author } + master_commit = @project.repository.commit('master') + @project.repository.merge(@user, master_commit.id, 'feature', commit_options) + commit = @project.repository.commit('feature') + service.new(@project, @user).execute(@oldrev, commit.id, 'refs/heads/feature') + reload_mrs + end + + it { expect(@merge_request.notes.last.note).to include('changed to merged') } + it { expect(@merge_request).to be_merged } + it { expect(@merge_request.diffs.length).to be > 0 } + it { expect(@fork_merge_request).to be_merged } + it { expect(@fork_merge_request.notes.last.note).to include('changed to merged') } + end + context 'push to fork repo source branch' do let(:refresh_service) { service.new(@fork_project, @user) } before do @@ -106,6 +125,27 @@ describe MergeRequests::RefreshService do it { expect(@fork_merge_request.notes).to be_empty } end + context 'push new branch that exists in a merge request' do + let(:refresh_service) { service.new(@fork_project, @user) } + + it 'refreshes the merge request' do + expect(refresh_service).to receive(:execute_hooks). + with(@fork_merge_request, 'update') + allow_any_instance_of(Repository).to receive(:merge_base).and_return(@oldrev) + + refresh_service.execute(Gitlab::Git::BLANK_SHA, @newrev, 'refs/heads/master') + reload_mrs + + expect(@merge_request.notes).to be_empty + expect(@merge_request).to be_open + + notes = @fork_merge_request.notes.reorder(:created_at).map(&:note) + expect(notes[0]).to include('Restored source branch `master`') + expect(notes[1]).to include('Added 4 commits') + expect(@fork_merge_request).to be_open + end + end + def reload_mrs @merge_request.reload @fork_merge_request.reload diff --git a/spec/services/projects/download_service_spec.rb b/spec/services/projects/download_service_spec.rb index f12e09c58c3..ddee2e62dfc 100644 --- a/spec/services/projects/download_service_spec.rb +++ b/spec/services/projects/download_service_spec.rb @@ -37,7 +37,6 @@ describe Projects::DownloadService do it { expect(@link_to_file).to have_key('url') } it { expect(@link_to_file).to have_key('is_image') } it { expect(@link_to_file['is_image']).to be true } - it { expect(@link_to_file['url']).to match("/#{@project.path_with_namespace}") } it { expect(@link_to_file['url']).to match('rails_sample.jpg') } it { expect(@link_to_file['alt']).to eq('rails_sample') } end @@ -52,7 +51,6 @@ describe Projects::DownloadService do it { expect(@link_to_file).to have_key('url') } it { expect(@link_to_file).to have_key('is_image') } it { expect(@link_to_file['is_image']).to be false } - it { expect(@link_to_file['url']).to match("/#{@project.path_with_namespace}") } it { expect(@link_to_file['url']).to match('doc_sample.txt') } it { expect(@link_to_file['alt']).to eq('doc_sample.txt') } end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index bb7da33b12e..47755bfc990 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -7,6 +7,8 @@ describe Projects::TransferService do context 'namespace -> namespace' do before do + allow_any_instance_of(Gitlab::UploadsTransfer). + to receive(:move_project).and_return(true) group.add_owner(user) @result = transfer_project(project, user, group) end diff --git a/spec/services/projects/upload_service_spec.rb b/spec/services/projects/upload_service_spec.rb index fa4ff6b01ad..1b1a80d1fe7 100644 --- a/spec/services/projects/upload_service_spec.rb +++ b/spec/services/projects/upload_service_spec.rb @@ -18,7 +18,6 @@ describe Projects::UploadService do it { expect(@link_to_file).to have_key(:is_image) } it { expect(@link_to_file).to have_value('banana_sample') } it { expect(@link_to_file[:is_image]).to equal(true) } - it { expect(@link_to_file[:url]).to match("/#{@project.path_with_namespace}") } it { expect(@link_to_file[:url]).to match('banana_sample.gif') } end @@ -34,7 +33,6 @@ describe Projects::UploadService do it { expect(@link_to_file).to have_value('dk') } it { expect(@link_to_file).to have_key(:is_image) } it { expect(@link_to_file[:is_image]).to equal(true) } - it { expect(@link_to_file[:url]).to match("/#{@project.path_with_namespace}") } it { expect(@link_to_file[:url]).to match('dk.png') } end @@ -49,7 +47,6 @@ describe Projects::UploadService do it { expect(@link_to_file).to have_key(:is_image) } it { expect(@link_to_file).to have_value('rails_sample') } it { expect(@link_to_file[:is_image]).to equal(true) } - it { expect(@link_to_file[:url]).to match("/#{@project.path_with_namespace}") } it { expect(@link_to_file[:url]).to match('rails_sample.jpg') } end @@ -64,7 +61,6 @@ describe Projects::UploadService do it { expect(@link_to_file).to have_key(:is_image) } it { expect(@link_to_file).to have_value('doc_sample.txt') } it { expect(@link_to_file[:is_image]).to equal(false) } - it { expect(@link_to_file[:url]).to match("/#{@project.path_with_namespace}") } it { expect(@link_to_file[:url]).to match('doc_sample.txt') } end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 2658576640c..a45130bd473 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -242,6 +242,18 @@ describe SystemNoteService do end end + describe '.change_branch_presence' do + subject { described_class.change_branch_presence(noteable, project, author, :source, 'feature', :delete) } + + it_behaves_like 'a system note' + + context 'when source branch deleted' do + it 'sets the note text' do + expect(subject.note).to eq "Deleted source branch `feature`" + end + end + end + describe '.cross_reference' do subject { described_class.cross_reference(noteable, mentioner, author) } diff --git a/spec/support/filter_spec_helper.rb b/spec/support/filter_spec_helper.rb index 203117aee70..97e5c270a59 100644 --- a/spec/support/filter_spec_helper.rb +++ b/spec/support/filter_spec_helper.rb @@ -29,12 +29,19 @@ module FilterSpecHelper # # Returns the Hash def pipeline_result(body, contexts = {}) - contexts.reverse_merge!(project: project) + contexts.reverse_merge!(project: project) if defined?(project) pipeline = HTML::Pipeline.new([described_class], contexts) pipeline.call(body) end + def reference_pipeline_result(body, contexts = {}) + contexts.reverse_merge!(project: project) if defined?(project) + + pipeline = HTML::Pipeline.new([described_class, Gitlab::Markdown::ReferenceGathererFilter], contexts) + pipeline.call(body) + end + # Modify a String reference to make it invalid # # Commit SHAs get reversed, IDs get incremented by 1, all other Strings get @@ -55,20 +62,6 @@ module FilterSpecHelper end end - # Stub CrossProjectReference#user_can_reference_project? to return true for - # the current test - def allow_cross_reference! - allow_any_instance_of(described_class). - to receive(:user_can_reference_project?).and_return(true) - end - - # Stub CrossProjectReference#user_can_reference_project? to return false for - # the current test - def disallow_cross_reference! - allow_any_instance_of(described_class). - to receive(:user_can_reference_project?).and_return(false) - end - # Shortcut to Rails' auto-generated routes helpers, to avoid including the # module def urls diff --git a/spec/support/markdown_feature.rb b/spec/support/markdown_feature.rb index 39a64391460..bedc1a7f1db 100644 --- a/spec/support/markdown_feature.rb +++ b/spec/support/markdown_feature.rb @@ -15,18 +15,17 @@ class MarkdownFeature end def group - unless @group - @group = create(:group) - @group.add_developer(user) + @group ||= create(:group).tap do |group| + group.add_developer(user) end - - @group end # Direct references ---------------------------------------------------------- def project - @project ||= create(:project) + @project ||= create(:project).tap do |project| + project.team << [user, :master] + end end def issue @@ -46,12 +45,10 @@ class MarkdownFeature end def commit_range - unless @commit_range + @commit_range ||= begin commit2 = project.commit('HEAD~3') - @commit_range = CommitRange.new("#{commit.id}...#{commit2.id}", project) + CommitRange.new("#{commit.id}...#{commit2.id}", project) end - - @commit_range end def simple_label @@ -65,13 +62,12 @@ class MarkdownFeature # Cross-references ----------------------------------------------------------- def xproject - unless @xproject + @xproject ||= begin namespace = create(:namespace, name: 'cross-reference') - @xproject = create(:project, namespace: namespace) - @xproject.team << [user, :developer] + create(:project, namespace: namespace) do |project| + project.team << [user, :developer] + end end - - @xproject end def xissue @@ -91,12 +87,10 @@ class MarkdownFeature end def xcommit_range - unless @xcommit_range + @xcommit_range ||= begin xcommit2 = xproject.commit('HEAD~2') - @xcommit_range = CommitRange.new("#{xcommit.id}...#{xcommit2.id}", xproject) + CommitRange.new("#{xcommit.id}...#{xcommit2.id}", xproject) end - - @xcommit_range end def raw_markdown diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb index e3de0afb448..3bb568f4d49 100644 --- a/spec/support/mentionable_shared_examples.rb +++ b/spec/support/mentionable_shared_examples.rb @@ -5,7 +5,7 @@ # - let(:set_mentionable_text) { lambda { |txt| "block that assigns txt to the subject's mentionable_text" } } def common_mentionable_setup - let(:project) { create :project } + let(:project) { subject.project } let(:author) { subject.author } let(:mentioned_issue) { create(:issue, project: project) } @@ -50,6 +50,8 @@ def common_mentionable_setup } extra_commits.each { |c| commitmap[c.short_id] = c } + allow(Project).to receive(:find).and_call_original + allow(Project).to receive(:find).with(project.id.to_s).and_return(project) allow(project.repository).to receive(:commit) { |sha| commitmap[sha] } set_mentionable_text.call(ref_string) @@ -65,7 +67,7 @@ shared_examples 'a mentionable' do it "extracts references from its reference property" do # De-duplicate and omit itself - refs = subject.references(project) + refs = subject.referenced_mentionables expect(refs.size).to eq(6) expect(refs).to include(mentioned_issue) expect(refs).to include(mentioned_mr) @@ -84,14 +86,7 @@ shared_examples 'a mentionable' do with(referenced, subject.local_reference, author) end - subject.create_cross_references!(project, author) - end - - it 'detects existing cross-references' do - SystemNoteService.cross_reference(mentioned_issue, subject.local_reference, author) - - expect(subject).to have_mentioned(mentioned_issue) - expect(subject).not_to have_mentioned(mentioned_mr) + subject.create_cross_references! end end @@ -143,6 +138,6 @@ shared_examples 'an editable mentionable' do end set_mentionable_text.call(new_text) - subject.create_new_cross_references!(project, author) + subject.create_new_cross_references!(author) end end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 3eab74ba986..787670e9297 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -9,7 +9,7 @@ module TestEnv 'flatten-dir' => 'e56497b', 'feature' => '0b4bc9a', 'feature_conflict' => 'bb5206f', - 'fix' => '12d65c8', + 'fix' => '48f0be4', 'improve/awesome' => '5937ac0', 'markdown' => '0ed8c6c', 'master' => '5937ac0', @@ -96,15 +96,15 @@ module TestEnv clone_url = "https://gitlab.com/gitlab-org/#{repo_name}.git" unless File.directory?(repo_path) - system(*%W(git clone -q #{clone_url} #{repo_path})) + system(*%W(#{Gitlab.config.git.bin_path} clone -q #{clone_url} #{repo_path})) end Dir.chdir(repo_path) do branch_sha.each do |branch, sha| # Try to reset without fetching to avoid using the network. - reset = %W(git update-ref refs/heads/#{branch} #{sha}) + reset = %W(#{Gitlab.config.git.bin_path} update-ref refs/heads/#{branch} #{sha}) unless system(*reset) - if system(*%w(git fetch origin)) + if system(*%W(#{Gitlab.config.git.bin_path} fetch origin)) unless system(*reset) raise 'The fetched test seed '\ 'does not contain the required revision.' @@ -117,7 +117,7 @@ module TestEnv end # We must copy bare repositories because we will push to them. - system(git_env, *%W(git clone -q --bare #{repo_path} #{repo_path_bare})) + system(git_env, *%W(#{Gitlab.config.git.bin_path} clone -q --bare #{repo_path} #{repo_path_bare})) end def copy_repo(project) diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index 3be7dd4e52b..386ac9c8372 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -55,6 +55,7 @@ describe 'gitlab:app namespace rake task' do expect(Rake::Task["gitlab:backup:db:restore"]).to receive(:invoke) expect(Rake::Task["gitlab:backup:repo:restore"]).to receive(:invoke) expect(Rake::Task["gitlab:backup:builds:restore"]).to receive(:invoke) + expect(Rake::Task["gitlab:backup:uploads: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 @@ -112,14 +113,14 @@ describe 'gitlab:app namespace rake task' do it 'should set correct permissions on the tar contents' do tar_contents, exit_status = Gitlab::Popen.popen( - %W{tar -tvf #{@backup_tar} db uploads repositories builds} + %W{tar -tvf #{@backup_tar} db uploads.tar.gz repositories builds.tar.gz} ) expect(exit_status).to eq(0) expect(tar_contents).to match('db/') - expect(tar_contents).to match('uploads/') + expect(tar_contents).to match('uploads.tar.gz') expect(tar_contents).to match('repositories/') - expect(tar_contents).to match('builds/') - expect(tar_contents).not_to match(/^.{4,9}[rwx].* (db|uploads|repositories|builds)\/$/) + expect(tar_contents).to match('builds.tar.gz') + expect(tar_contents).not_to match(/^.{4,9}[rwx].* (database.sql.gz|uploads.tar.gz|repositories|builds.tar.gz)\/$/) end it 'should delete temp directories' do @@ -160,12 +161,12 @@ describe 'gitlab:app namespace rake task' do it "does not contain skipped item" do tar_contents, _exit_status = Gitlab::Popen.popen( - %W{tar -tvf #{@backup_tar} db uploads repositories builds} + %W{tar -tvf #{@backup_tar} db uploads.tar.gz repositories builds.tar.gz} ) expect(tar_contents).to match('db/') - expect(tar_contents).to match('uploads/') - expect(tar_contents).to match('builds/') + expect(tar_contents).to match('uploads.tar.gz') + expect(tar_contents).to match('builds.tar.gz') expect(tar_contents).not_to match('repositories/') end diff --git a/spec/workers/repository_archive_worker_spec.rb b/spec/workers/repository_archive_worker_spec.rb deleted file mode 100644 index a914d0ac8dc..00000000000 --- a/spec/workers/repository_archive_worker_spec.rb +++ /dev/null @@ -1,79 +0,0 @@ -require 'spec_helper' - -describe RepositoryArchiveWorker do - let(:project) { create(:project) } - subject { RepositoryArchiveWorker.new } - - before do - allow(Project).to receive(:find).and_return(project) - end - - describe "#perform" do - it "cleans old archives" do - expect(project.repository).to receive(:clean_old_archives) - - subject.perform(project.id, "master", "zip") - end - - context "when the repository doesn't have an archive file path" do - before do - allow(project.repository).to receive(:archive_file_path).and_return(nil) - end - - it "doesn't archive the repo" do - expect(project.repository).not_to receive(:archive_repo) - - subject.perform(project.id, "master", "zip") - end - end - - context "when the repository has an archive file path" do - let(:file_path) { "/archive.zip" } - let(:pid_file_path) { "/archive.zip.pid" } - - before do - allow(project.repository).to receive(:archive_file_path).and_return(file_path) - allow(project.repository).to receive(:archive_pid_file_path).and_return(pid_file_path) - end - - context "when the archive file already exists" do - before do - allow(File).to receive(:exist?).with(file_path).and_return(true) - end - - it "doesn't archive the repo" do - expect(project.repository).not_to receive(:archive_repo) - - subject.perform(project.id, "master", "zip") - end - end - - context "when the archive file doesn't exist yet" do - before do - allow(File).to receive(:exist?).with(file_path).and_return(false) - allow(File).to receive(:exist?).with(pid_file_path).and_return(true) - end - - context "when the archive pid file doesn't exist yet" do - before do - allow(File).to receive(:exist?).with(pid_file_path).and_return(false) - end - - it "archives the repo" do - expect(project.repository).to receive(:archive_repo) - - subject.perform(project.id, "master", "zip") - end - end - - context "when the archive pid file already exists" do - it "doesn't archive the repo" do - expect(project.repository).not_to receive(:archive_repo) - - subject.perform(project.id, "master", "zip") - end - end - end - end - end -end diff --git a/spec/workers/stuck_ci_builds_worker_spec.rb b/spec/workers/stuck_ci_builds_worker_spec.rb new file mode 100644 index 00000000000..f9d87d97014 --- /dev/null +++ b/spec/workers/stuck_ci_builds_worker_spec.rb @@ -0,0 +1,44 @@ +require "spec_helper" + +describe StuckCiBuildsWorker do + let!(:build) { create :ci_build } + + subject do + build.reload + build.status + end + + %w(pending running).each do |status| + context "#{status} build" do + before do + build.update!(status: status) + end + + it 'gets dropped if it was updated over 2 days ago' do + build.update!(updated_at: 2.day.ago) + StuckCiBuildsWorker.new.perform + is_expected.to eq('failed') + end + + it "is still #{status}" do + build.update!(updated_at: 1.minute.ago) + StuckCiBuildsWorker.new.perform + is_expected.to eq(status) + end + end + end + + %w(success failed canceled).each do |status| + context "#{status} build" do + before do + build.update!(status: status) + end + + it "is still #{status}" do + build.update!(updated_at: 2.day.ago) + StuckCiBuildsWorker.new.perform + is_expected.to eq(status) + end + end + end +end |