From 5069682d8ed892705ec1a933554cc4060e5691af Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Tue, 25 Apr 2017 13:28:55 +0100 Subject: Enable RSpec/FilePath cop - Ignore JS fixtures - Ignore qa directory - Rewrite concern specs to put concern name first --- .rubocop.yml | 10 +- spec/controllers/blob_controller_spec.rb | 67 ------ .../personal_access_tokens_controller_spec.rb | 56 +++++ .../profiles/personal_access_tokens_spec.rb | 56 ----- spec/controllers/projects/blob_controller_spec.rb | 51 +++++ spec/controllers/projects/todo_controller_spec.rb | 144 ------------ spec/controllers/projects/todos_controller_spec.rb | 144 ++++++++++++ spec/lib/git_ref_validator_spec.rb | 25 --- spec/lib/gitlab/backend/shell_spec.rb | 94 -------- spec/lib/gitlab/checks/force_push_spec.rb | 6 +- spec/lib/gitlab/git_ref_validator_spec.rb | 25 +++ spec/lib/gitlab/health_checks/db_check_spec.rb | 6 + .../gitlab/health_checks/fs_shards_check_spec.rb | 127 +++++++++++ spec/lib/gitlab/health_checks/redis_check_spec.rb | 6 + .../gitlab/health_checks/simple_check_shared.rb | 66 ++++++ spec/lib/gitlab/healthchecks/db_check_spec.rb | 6 - .../gitlab/healthchecks/fs_shards_check_spec.rb | 127 ----------- spec/lib/gitlab/healthchecks/redis_check_spec.rb | 6 - .../lib/gitlab/healthchecks/simple_check_shared.rb | 66 ------ .../gitlab/import_export/wiki_repo_bundler_spec.rb | 27 --- .../gitlab/import_export/wiki_repo_saver_spec.rb | 27 +++ spec/lib/gitlab/shell_spec.rb | 94 ++++++++ spec/lib/light_url_builder_spec.rb | 119 ---------- spec/mailers/emails/merge_requests_spec.rb | 2 +- spec/mailers/emails/profile_spec.rb | 146 ++++++------ spec/migrations/active_record/schema_spec.rb | 23 ++ spec/migrations/schema_spec.rb | 23 -- spec/models/concerns/awardable_spec.rb | 4 +- spec/models/concerns/discussion_on_diff_spec.rb | 6 +- spec/models/concerns/issuable_spec.rb | 35 +-- spec/models/concerns/noteable_spec.rb | 2 +- spec/models/concerns/relative_positioning_spec.rb | 2 +- spec/models/concerns/spammable_spec.rb | 4 +- spec/models/concerns/strip_attribute_spec.rb | 2 +- .../pipeline_email_service_spec.rb | 175 --------------- .../pipelines_email_service_spec.rb | 175 +++++++++++++++ spec/policies/issue_policy_spec.rb | 246 ++++++++++++++------- spec/policies/issues_policy_spec.rb | 193 ---------------- spec/requests/api/api_internal_helpers_spec.rb | 32 --- spec/requests/api/doorkeeper_access_spec.rb | 2 +- spec/requests/api/helpers/internal_helpers_spec.rb | 32 +++ spec/requests/api/oauth_tokens_spec.rb | 2 +- spec/routing/environments_spec.rb | 2 +- spec/routing/notifications_routing_spec.rb | 14 +- spec/serializers/analytics_generic_entity_spec.rb | 39 ---- spec/serializers/analytics_issue_entity_spec.rb | 39 ++++ spec/services/issues/resolve_discussions_spec.rb | 24 +- .../resolved_discussion_notification_service.rb | 46 ---- ...esolved_discussion_notification_service_spec.rb | 46 ++++ spec/workers/pipeline_proccess_worker_spec.rb | 22 -- spec/workers/pipeline_process_worker_spec.rb | 22 ++ 51 files changed, 1236 insertions(+), 1479 deletions(-) delete mode 100644 spec/controllers/blob_controller_spec.rb create mode 100644 spec/controllers/profiles/personal_access_tokens_controller_spec.rb delete mode 100644 spec/controllers/profiles/personal_access_tokens_spec.rb delete mode 100644 spec/controllers/projects/todo_controller_spec.rb create mode 100644 spec/controllers/projects/todos_controller_spec.rb delete mode 100644 spec/lib/git_ref_validator_spec.rb delete mode 100644 spec/lib/gitlab/backend/shell_spec.rb create mode 100644 spec/lib/gitlab/git_ref_validator_spec.rb create mode 100644 spec/lib/gitlab/health_checks/db_check_spec.rb create mode 100644 spec/lib/gitlab/health_checks/fs_shards_check_spec.rb create mode 100644 spec/lib/gitlab/health_checks/redis_check_spec.rb create mode 100644 spec/lib/gitlab/health_checks/simple_check_shared.rb delete mode 100644 spec/lib/gitlab/healthchecks/db_check_spec.rb delete mode 100644 spec/lib/gitlab/healthchecks/fs_shards_check_spec.rb delete mode 100644 spec/lib/gitlab/healthchecks/redis_check_spec.rb delete mode 100644 spec/lib/gitlab/healthchecks/simple_check_shared.rb delete mode 100644 spec/lib/gitlab/import_export/wiki_repo_bundler_spec.rb create mode 100644 spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb create mode 100644 spec/lib/gitlab/shell_spec.rb delete mode 100644 spec/lib/light_url_builder_spec.rb create mode 100644 spec/migrations/active_record/schema_spec.rb delete mode 100644 spec/migrations/schema_spec.rb delete mode 100644 spec/models/project_services/pipeline_email_service_spec.rb create mode 100644 spec/models/project_services/pipelines_email_service_spec.rb delete mode 100644 spec/policies/issues_policy_spec.rb delete mode 100644 spec/requests/api/api_internal_helpers_spec.rb create mode 100644 spec/requests/api/helpers/internal_helpers_spec.rb delete mode 100644 spec/serializers/analytics_generic_entity_spec.rb create mode 100644 spec/serializers/analytics_issue_entity_spec.rb delete mode 100644 spec/services/merge_requests/resolved_discussion_notification_service.rb create mode 100644 spec/services/merge_requests/resolved_discussion_notification_service_spec.rb delete mode 100644 spec/workers/pipeline_proccess_worker_spec.rb create mode 100644 spec/workers/pipeline_process_worker_spec.rb diff --git a/.rubocop.yml b/.rubocop.yml index e5549b64503..85d116ec06c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -983,10 +983,12 @@ RSpec/ExpectActual: # Checks the file and folder naming of the spec file. RSpec/FilePath: - Enabled: false - CustomTransform: - RuboCop: rubocop - RSpec: rspec + Enabled: true + IgnoreMethods: true + Exclude: + - 'qa/**/*' + - 'spec/javascripts/fixtures/*' + - 'spec/requests/api/v3/*' # Checks if there are focused specs. RSpec/Focus: diff --git a/spec/controllers/blob_controller_spec.rb b/spec/controllers/blob_controller_spec.rb deleted file mode 100644 index 44e011fd3a8..00000000000 --- a/spec/controllers/blob_controller_spec.rb +++ /dev/null @@ -1,67 +0,0 @@ -require 'spec_helper' - -describe Projects::BlobController do - let(:project) { create(:project, :repository) } - let(:user) { create(:user) } - - before do - sign_in(user) - - project.team << [user, :master] - - allow(project).to receive(:branches).and_return(['master', 'foo/bar/baz']) - allow(project).to receive(:tags).and_return(['v1.0.0', 'v2.0.0']) - controller.instance_variable_set(:@project, project) - end - - describe "GET show" do - render_views - - before do - get(:show, - namespace_id: project.namespace, - project_id: project, - id: id) - end - - context "valid branch, valid file" do - let(:id) { 'master/README.md' } - it { is_expected.to respond_with(:success) } - end - - context "valid branch, invalid file" do - let(:id) { 'master/invalid-path.rb' } - it { is_expected.to respond_with(:not_found) } - end - - context "invalid branch, valid file" do - let(:id) { 'invalid-branch/README.md' } - it { is_expected.to respond_with(:not_found) } - end - - context "binary file" do - let(:id) { 'binary-encoding/encoding/binary-1.bin' } - it { is_expected.to respond_with(:success) } - end - end - - describe 'GET show with tree path' do - render_views - - before do - get(:show, - namespace_id: project.namespace, - project_id: project, - id: id) - controller.instance_variable_set(:@blob, nil) - end - - context 'redirect to tree' do - let(:id) { 'markdown/doc' } - it 'redirects' do - expect(subject). - to redirect_to("/#{project.path_with_namespace}/tree/markdown/doc") - end - end - end -end diff --git a/spec/controllers/profiles/personal_access_tokens_controller_spec.rb b/spec/controllers/profiles/personal_access_tokens_controller_spec.rb new file mode 100644 index 00000000000..98a43e278b2 --- /dev/null +++ b/spec/controllers/profiles/personal_access_tokens_controller_spec.rb @@ -0,0 +1,56 @@ +require 'spec_helper' + +describe Profiles::PersonalAccessTokensController do + let(:user) { create(:user) } + let(:token_attributes) { attributes_for(:personal_access_token) } + + before { sign_in(user) } + + describe '#create' do + def created_token + PersonalAccessToken.order(:created_at).last + end + + it "allows creation of a token with scopes" do + name = 'My PAT' + scopes = %w[api read_user] + + post :create, personal_access_token: token_attributes.merge(scopes: scopes, name: name) + + expect(created_token).not_to be_nil + expect(created_token.name).to eq(name) + expect(created_token.scopes).to eq(scopes) + expect(PersonalAccessToken.active).to include(created_token) + end + + it "allows creation of a token with an expiry date" do + expires_at = 5.days.from_now.to_date + + post :create, personal_access_token: token_attributes.merge(expires_at: expires_at) + + expect(created_token).not_to be_nil + expect(created_token.expires_at).to eq(expires_at) + end + end + + describe '#index' do + let!(:active_personal_access_token) { create(:personal_access_token, user: user) } + let!(:inactive_personal_access_token) { create(:personal_access_token, :revoked, user: user) } + let!(:impersonation_personal_access_token) { create(:personal_access_token, :impersonation, user: user) } + + before { get :index } + + it "retrieves active personal access tokens" do + expect(assigns(:active_personal_access_tokens)).to include(active_personal_access_token) + end + + it "retrieves inactive personal access tokens" do + expect(assigns(:inactive_personal_access_tokens)).to include(inactive_personal_access_token) + end + + it "does not retrieve impersonation personal access tokens" do + expect(assigns(:active_personal_access_tokens)).not_to include(impersonation_personal_access_token) + expect(assigns(:inactive_personal_access_tokens)).not_to include(impersonation_personal_access_token) + end + end +end diff --git a/spec/controllers/profiles/personal_access_tokens_spec.rb b/spec/controllers/profiles/personal_access_tokens_spec.rb deleted file mode 100644 index 98a43e278b2..00000000000 --- a/spec/controllers/profiles/personal_access_tokens_spec.rb +++ /dev/null @@ -1,56 +0,0 @@ -require 'spec_helper' - -describe Profiles::PersonalAccessTokensController do - let(:user) { create(:user) } - let(:token_attributes) { attributes_for(:personal_access_token) } - - before { sign_in(user) } - - describe '#create' do - def created_token - PersonalAccessToken.order(:created_at).last - end - - it "allows creation of a token with scopes" do - name = 'My PAT' - scopes = %w[api read_user] - - post :create, personal_access_token: token_attributes.merge(scopes: scopes, name: name) - - expect(created_token).not_to be_nil - expect(created_token.name).to eq(name) - expect(created_token.scopes).to eq(scopes) - expect(PersonalAccessToken.active).to include(created_token) - end - - it "allows creation of a token with an expiry date" do - expires_at = 5.days.from_now.to_date - - post :create, personal_access_token: token_attributes.merge(expires_at: expires_at) - - expect(created_token).not_to be_nil - expect(created_token.expires_at).to eq(expires_at) - end - end - - describe '#index' do - let!(:active_personal_access_token) { create(:personal_access_token, user: user) } - let!(:inactive_personal_access_token) { create(:personal_access_token, :revoked, user: user) } - let!(:impersonation_personal_access_token) { create(:personal_access_token, :impersonation, user: user) } - - before { get :index } - - it "retrieves active personal access tokens" do - expect(assigns(:active_personal_access_tokens)).to include(active_personal_access_token) - end - - it "retrieves inactive personal access tokens" do - expect(assigns(:inactive_personal_access_tokens)).to include(inactive_personal_access_token) - end - - it "does not retrieve impersonation personal access tokens" do - expect(assigns(:active_personal_access_tokens)).not_to include(impersonation_personal_access_token) - expect(assigns(:inactive_personal_access_tokens)).not_to include(impersonation_personal_access_token) - end - end -end diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 0fd09d156c4..3b3caa9d3e6 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -3,6 +3,57 @@ require 'rails_helper' describe Projects::BlobController do let(:project) { create(:project, :public, :repository) } + describe "GET show" do + render_views + + context 'with file path' do + before do + get(:show, + namespace_id: project.namespace, + project_id: project, + id: id) + end + + context "valid branch, valid file" do + let(:id) { 'master/README.md' } + it { is_expected.to respond_with(:success) } + end + + context "valid branch, invalid file" do + let(:id) { 'master/invalid-path.rb' } + it { is_expected.to respond_with(:not_found) } + end + + context "invalid branch, valid file" do + let(:id) { 'invalid-branch/README.md' } + it { is_expected.to respond_with(:not_found) } + end + + context "binary file" do + let(:id) { 'binary-encoding/encoding/binary-1.bin' } + it { is_expected.to respond_with(:success) } + end + end + + context 'with tree path' do + before do + get(:show, + namespace_id: project.namespace, + project_id: project, + id: id) + controller.instance_variable_set(:@blob, nil) + end + + context 'redirect to tree' do + let(:id) { 'markdown/doc' } + it 'redirects' do + expect(subject). + to redirect_to("/#{project.path_with_namespace}/tree/markdown/doc") + end + end + end + end + describe 'GET diff' do let(:user) { create(:user) } diff --git a/spec/controllers/projects/todo_controller_spec.rb b/spec/controllers/projects/todo_controller_spec.rb deleted file mode 100644 index c5a4153d991..00000000000 --- a/spec/controllers/projects/todo_controller_spec.rb +++ /dev/null @@ -1,144 +0,0 @@ -require('spec_helper') - -describe Projects::TodosController do - let(:user) { create(:user) } - let(:project) { create(:empty_project) } - let(:issue) { create(:issue, project: project) } - let(:merge_request) { create(:merge_request, source_project: project) } - - context 'Issues' do - describe 'POST create' do - def go - post :create, - namespace_id: project.namespace, - project_id: project, - issuable_id: issue.id, - issuable_type: 'issue', - format: 'html' - end - - context 'when authorized' do - before do - sign_in(user) - project.team << [user, :developer] - end - - it 'creates todo for issue' do - expect do - go - end.to change { user.todos.count }.by(1) - - expect(response).to have_http_status(200) - end - - it 'returns todo path and pending count' do - go - - expect(response).to have_http_status(200) - expect(json_response['count']).to eq 1 - expect(json_response['delete_path']).to match(/\/dashboard\/todos\/\d{1}/) - end - end - - context 'when not authorized for project' do - it 'does not create todo for issue that user has no access to' do - sign_in(user) - expect do - go - end.to change { user.todos.count }.by(0) - - expect(response).to have_http_status(404) - end - - it 'does not create todo for issue when user not logged in' do - expect do - go - end.to change { user.todos.count }.by(0) - - expect(response).to have_http_status(302) - end - end - - context 'when not authorized for issue' do - before do - project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) - project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) - sign_in(user) - end - - it "doesn't create todo" do - expect{ go }.not_to change { user.todos.count } - expect(response).to have_http_status(404) - end - end - end - end - - context 'Merge Requests' do - describe 'POST create' do - def go - post :create, - namespace_id: project.namespace, - project_id: project, - issuable_id: merge_request.id, - issuable_type: 'merge_request', - format: 'html' - end - - context 'when authorized' do - before do - sign_in(user) - project.team << [user, :developer] - end - - it 'creates todo for merge request' do - expect do - go - end.to change { user.todos.count }.by(1) - - expect(response).to have_http_status(200) - end - - it 'returns todo path and pending count' do - go - - expect(response).to have_http_status(200) - expect(json_response['count']).to eq 1 - expect(json_response['delete_path']).to match(/\/dashboard\/todos\/\d{1}/) - end - end - - context 'when not authorized for project' do - it 'does not create todo for merge request user has no access to' do - sign_in(user) - expect do - go - end.to change { user.todos.count }.by(0) - - expect(response).to have_http_status(404) - end - - it 'does not create todo for merge request user has no access to' do - expect do - go - end.to change { user.todos.count }.by(0) - - expect(response).to have_http_status(302) - end - end - - context 'when not authorized for merge_request' do - before do - project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) - project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) - sign_in(user) - end - - it "doesn't create todo" do - expect{ go }.not_to change { user.todos.count } - expect(response).to have_http_status(404) - end - end - end - end -end diff --git a/spec/controllers/projects/todos_controller_spec.rb b/spec/controllers/projects/todos_controller_spec.rb new file mode 100644 index 00000000000..c5a4153d991 --- /dev/null +++ b/spec/controllers/projects/todos_controller_spec.rb @@ -0,0 +1,144 @@ +require('spec_helper') + +describe Projects::TodosController do + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + let(:issue) { create(:issue, project: project) } + let(:merge_request) { create(:merge_request, source_project: project) } + + context 'Issues' do + describe 'POST create' do + def go + post :create, + namespace_id: project.namespace, + project_id: project, + issuable_id: issue.id, + issuable_type: 'issue', + format: 'html' + end + + context 'when authorized' do + before do + sign_in(user) + project.team << [user, :developer] + end + + it 'creates todo for issue' do + expect do + go + end.to change { user.todos.count }.by(1) + + expect(response).to have_http_status(200) + end + + it 'returns todo path and pending count' do + go + + expect(response).to have_http_status(200) + expect(json_response['count']).to eq 1 + expect(json_response['delete_path']).to match(/\/dashboard\/todos\/\d{1}/) + end + end + + context 'when not authorized for project' do + it 'does not create todo for issue that user has no access to' do + sign_in(user) + expect do + go + end.to change { user.todos.count }.by(0) + + expect(response).to have_http_status(404) + end + + it 'does not create todo for issue when user not logged in' do + expect do + go + end.to change { user.todos.count }.by(0) + + expect(response).to have_http_status(302) + end + end + + context 'when not authorized for issue' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + sign_in(user) + end + + it "doesn't create todo" do + expect{ go }.not_to change { user.todos.count } + expect(response).to have_http_status(404) + end + end + end + end + + context 'Merge Requests' do + describe 'POST create' do + def go + post :create, + namespace_id: project.namespace, + project_id: project, + issuable_id: merge_request.id, + issuable_type: 'merge_request', + format: 'html' + end + + context 'when authorized' do + before do + sign_in(user) + project.team << [user, :developer] + end + + it 'creates todo for merge request' do + expect do + go + end.to change { user.todos.count }.by(1) + + expect(response).to have_http_status(200) + end + + it 'returns todo path and pending count' do + go + + expect(response).to have_http_status(200) + expect(json_response['count']).to eq 1 + expect(json_response['delete_path']).to match(/\/dashboard\/todos\/\d{1}/) + end + end + + context 'when not authorized for project' do + it 'does not create todo for merge request user has no access to' do + sign_in(user) + expect do + go + end.to change { user.todos.count }.by(0) + + expect(response).to have_http_status(404) + end + + it 'does not create todo for merge request user has no access to' do + expect do + go + end.to change { user.todos.count }.by(0) + + expect(response).to have_http_status(302) + end + end + + context 'when not authorized for merge_request' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + sign_in(user) + end + + it "doesn't create todo" do + expect{ go }.not_to change { user.todos.count } + expect(response).to have_http_status(404) + end + end + end + end +end diff --git a/spec/lib/git_ref_validator_spec.rb b/spec/lib/git_ref_validator_spec.rb deleted file mode 100644 index cc8daa535d6..00000000000 --- a/spec/lib/git_ref_validator_spec.rb +++ /dev/null @@ -1,25 +0,0 @@ -require 'spec_helper' - -describe Gitlab::GitRefValidator, lib: true do - it { expect(Gitlab::GitRefValidator.validate('feature/new')).to be_truthy } - it { expect(Gitlab::GitRefValidator.validate('implement_@all')).to be_truthy } - it { expect(Gitlab::GitRefValidator.validate('my_new_feature')).to be_truthy } - it { expect(Gitlab::GitRefValidator.validate('#1')).to be_truthy } - it { expect(Gitlab::GitRefValidator.validate('feature/refs/heads/foo')).to be_truthy } - it { expect(Gitlab::GitRefValidator.validate('feature/~new/')).to be_falsey } - it { expect(Gitlab::GitRefValidator.validate('feature/^new/')).to be_falsey } - it { expect(Gitlab::GitRefValidator.validate('feature/:new/')).to be_falsey } - it { expect(Gitlab::GitRefValidator.validate('feature/?new/')).to be_falsey } - it { expect(Gitlab::GitRefValidator.validate('feature/*new/')).to be_falsey } - it { expect(Gitlab::GitRefValidator.validate('feature/[new/')).to be_falsey } - it { expect(Gitlab::GitRefValidator.validate('feature/new/')).to be_falsey } - it { expect(Gitlab::GitRefValidator.validate('feature/new.')).to be_falsey } - it { expect(Gitlab::GitRefValidator.validate('feature\@{')).to be_falsey } - it { expect(Gitlab::GitRefValidator.validate('feature\new')).to be_falsey } - it { expect(Gitlab::GitRefValidator.validate('feature//new')).to be_falsey } - it { expect(Gitlab::GitRefValidator.validate('feature new')).to be_falsey } - it { expect(Gitlab::GitRefValidator.validate('refs/heads/')).to be_falsey } - it { expect(Gitlab::GitRefValidator.validate('refs/remotes/')).to be_falsey } - it { expect(Gitlab::GitRefValidator.validate('refs/heads/feature')).to be_falsey } - it { expect(Gitlab::GitRefValidator.validate('refs/remotes/origin')).to be_falsey } -end diff --git a/spec/lib/gitlab/backend/shell_spec.rb b/spec/lib/gitlab/backend/shell_spec.rb deleted file mode 100644 index 6675d26734e..00000000000 --- a/spec/lib/gitlab/backend/shell_spec.rb +++ /dev/null @@ -1,94 +0,0 @@ -require 'spec_helper' -require 'stringio' - -describe Gitlab::Shell, lib: true do - let(:project) { double('Project', id: 7, path: 'diaspora') } - let(:gitlab_shell) { Gitlab::Shell.new } - - before do - allow(Project).to receive(:find).and_return(project) - end - - it { is_expected.to respond_to :add_key } - it { is_expected.to respond_to :remove_key } - it { is_expected.to respond_to :add_repository } - it { is_expected.to respond_to :remove_repository } - it { is_expected.to respond_to :fork_repository } - it { is_expected.to respond_to :add_namespace } - it { is_expected.to respond_to :rm_namespace } - it { is_expected.to respond_to :mv_namespace } - it { is_expected.to respond_to :exists? } - - it { expect(gitlab_shell.url_to_repo('diaspora')).to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + "diaspora.git") } - - describe 'memoized secret_token' do - let(:secret_file) { 'tmp/tests/.secret_shell_test' } - let(:link_file) { 'tmp/tests/shell-secret-test/.gitlab_shell_secret' } - - before do - allow(Gitlab.config.gitlab_shell).to receive(:secret_file).and_return(secret_file) - allow(Gitlab.config.gitlab_shell).to receive(:path).and_return('tmp/tests/shell-secret-test') - FileUtils.mkdir('tmp/tests/shell-secret-test') - Gitlab::Shell.ensure_secret_token! - end - - after do - FileUtils.rm_rf('tmp/tests/shell-secret-test') - FileUtils.rm_rf(secret_file) - end - - it 'creates and links the secret token file' do - secret_token = Gitlab::Shell.secret_token - - expect(File.exist?(secret_file)).to be(true) - expect(File.read(secret_file).chomp).to eq(secret_token) - expect(File.symlink?(link_file)).to be(true) - expect(File.readlink(link_file)).to eq(secret_file) - end - end - - describe '#add_key' do - it 'removes trailing garbage' do - allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) - expect(Gitlab::Utils).to receive(:system_silent).with( - [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar'] - ) - - gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') - end - end - - describe Gitlab::Shell::KeyAdder, lib: true do - describe '#add_key' do - it 'removes trailing garbage' do - io = spy(:io) - adder = described_class.new(io) - - adder.add_key('key-42', "ssh-rsa foo bar\tbaz") - - expect(io).to have_received(:puts).with("key-42\tssh-rsa foo") - end - - it 'handles multiple spaces in the key' do - io = spy(:io) - adder = described_class.new(io) - - adder.add_key('key-42', "ssh-rsa foo") - - expect(io).to have_received(:puts).with("key-42\tssh-rsa foo") - end - - it 'raises an exception if the key contains a tab' do - expect do - described_class.new(StringIO.new).add_key('key-42', "ssh-rsa\tfoobar") - end.to raise_error(Gitlab::Shell::Error) - end - - it 'raises an exception if the key contains a newline' do - expect do - described_class.new(StringIO.new).add_key('key-42', "ssh-rsa foobar\nssh-rsa pawned") - end.to raise_error(Gitlab::Shell::Error) - end - end - end -end diff --git a/spec/lib/gitlab/checks/force_push_spec.rb b/spec/lib/gitlab/checks/force_push_spec.rb index 7a84bbebd02..bc66ce83d4a 100644 --- a/spec/lib/gitlab/checks/force_push_spec.rb +++ b/spec/lib/gitlab/checks/force_push_spec.rb @@ -1,19 +1,19 @@ require 'spec_helper' -describe Gitlab::Checks::ChangeAccess, lib: true do +describe Gitlab::Checks::ForcePush, lib: true do let(:project) { create(:project, :repository) } context "exit code checking" do it "does not raise a runtime error if the `popen` call to git returns a zero exit code" do allow(Gitlab::Popen).to receive(:popen).and_return(['normal output', 0]) - expect { Gitlab::Checks::ForcePush.force_push?(project, 'oldrev', 'newrev') }.not_to raise_error + expect { described_class.force_push?(project, 'oldrev', 'newrev') }.not_to raise_error end it "raises a runtime error if the `popen` call to git returns a non-zero exit code" do allow(Gitlab::Popen).to receive(:popen).and_return(['error', 1]) - expect { Gitlab::Checks::ForcePush.force_push?(project, 'oldrev', 'newrev') }.to raise_error(RuntimeError) + expect { described_class.force_push?(project, 'oldrev', 'newrev') }.to raise_error(RuntimeError) end end end diff --git a/spec/lib/gitlab/git_ref_validator_spec.rb b/spec/lib/gitlab/git_ref_validator_spec.rb new file mode 100644 index 00000000000..cc8daa535d6 --- /dev/null +++ b/spec/lib/gitlab/git_ref_validator_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe Gitlab::GitRefValidator, lib: true do + it { expect(Gitlab::GitRefValidator.validate('feature/new')).to be_truthy } + it { expect(Gitlab::GitRefValidator.validate('implement_@all')).to be_truthy } + it { expect(Gitlab::GitRefValidator.validate('my_new_feature')).to be_truthy } + it { expect(Gitlab::GitRefValidator.validate('#1')).to be_truthy } + it { expect(Gitlab::GitRefValidator.validate('feature/refs/heads/foo')).to be_truthy } + it { expect(Gitlab::GitRefValidator.validate('feature/~new/')).to be_falsey } + it { expect(Gitlab::GitRefValidator.validate('feature/^new/')).to be_falsey } + it { expect(Gitlab::GitRefValidator.validate('feature/:new/')).to be_falsey } + it { expect(Gitlab::GitRefValidator.validate('feature/?new/')).to be_falsey } + it { expect(Gitlab::GitRefValidator.validate('feature/*new/')).to be_falsey } + it { expect(Gitlab::GitRefValidator.validate('feature/[new/')).to be_falsey } + it { expect(Gitlab::GitRefValidator.validate('feature/new/')).to be_falsey } + it { expect(Gitlab::GitRefValidator.validate('feature/new.')).to be_falsey } + it { expect(Gitlab::GitRefValidator.validate('feature\@{')).to be_falsey } + it { expect(Gitlab::GitRefValidator.validate('feature\new')).to be_falsey } + it { expect(Gitlab::GitRefValidator.validate('feature//new')).to be_falsey } + it { expect(Gitlab::GitRefValidator.validate('feature new')).to be_falsey } + it { expect(Gitlab::GitRefValidator.validate('refs/heads/')).to be_falsey } + it { expect(Gitlab::GitRefValidator.validate('refs/remotes/')).to be_falsey } + it { expect(Gitlab::GitRefValidator.validate('refs/heads/feature')).to be_falsey } + it { expect(Gitlab::GitRefValidator.validate('refs/remotes/origin')).to be_falsey } +end diff --git a/spec/lib/gitlab/health_checks/db_check_spec.rb b/spec/lib/gitlab/health_checks/db_check_spec.rb new file mode 100644 index 00000000000..33c6c24449c --- /dev/null +++ b/spec/lib/gitlab/health_checks/db_check_spec.rb @@ -0,0 +1,6 @@ +require 'spec_helper' +require_relative './simple_check_shared' + +describe Gitlab::HealthChecks::DbCheck do + include_examples 'simple_check', 'db_ping', 'Db', '1' +end diff --git a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb b/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb new file mode 100644 index 00000000000..4cd8cf313a5 --- /dev/null +++ b/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb @@ -0,0 +1,127 @@ +require 'spec_helper' + +describe Gitlab::HealthChecks::FsShardsCheck do + let(:metric_class) { Gitlab::HealthChecks::Metric } + let(:result_class) { Gitlab::HealthChecks::Result } + let(:repository_storages) { [:default] } + let(:tmp_dir) { Dir.mktmpdir } + + let(:storages_paths) do + { + default: { path: tmp_dir } + }.with_indifferent_access + end + + before do + allow(described_class).to receive(:repository_storages) { repository_storages } + allow(described_class).to receive(:storages_paths) { storages_paths } + end + + after do + FileUtils.remove_entry_secure(tmp_dir) if Dir.exist?(tmp_dir) + end + + shared_examples 'filesystem checks' do + describe '#readiness' do + subject { described_class.readiness } + + context 'storage points to not existing folder' do + let(:storages_paths) do + { + default: { path: 'tmp/this/path/doesnt/exist' } + }.with_indifferent_access + end + + it { is_expected.to include(result_class.new(false, 'cannot stat storage', shard: :default)) } + end + + context 'storage points to directory that has both read and write rights' do + before do + FileUtils.chmod_R(0755, tmp_dir) + end + + it { is_expected.to include(result_class.new(true, nil, shard: :default)) } + + it 'cleans up files used for testing' do + expect(described_class).to receive(:storage_write_test).with(any_args).and_call_original + + subject + + expect(Dir.entries(tmp_dir).count).to eq(2) + end + + context 'read test fails' do + before do + allow(described_class).to receive(:storage_read_test).with(any_args).and_return(false) + end + + it { is_expected.to include(result_class.new(false, 'cannot read from storage', shard: :default)) } + end + + context 'write test fails' do + before do + allow(described_class).to receive(:storage_write_test).with(any_args).and_return(false) + end + + it { is_expected.to include(result_class.new(false, 'cannot write to storage', shard: :default)) } + end + end + end + + describe '#metrics' do + subject { described_class.metrics } + + context 'storage points to not existing folder' do + let(:storages_paths) do + { + default: { path: 'tmp/this/path/doesnt/exist' } + }.with_indifferent_access + end + + it { is_expected.to include(metric_class.new(:filesystem_accessible, 0, shard: :default)) } + it { is_expected.to include(metric_class.new(:filesystem_readable, 0, shard: :default)) } + it { is_expected.to include(metric_class.new(:filesystem_writable, 0, shard: :default)) } + + it { is_expected.to include(have_attributes(name: :filesystem_access_latency, value: be > 0, labels: { shard: :default })) } + it { is_expected.to include(have_attributes(name: :filesystem_read_latency, value: be > 0, labels: { shard: :default })) } + it { is_expected.to include(have_attributes(name: :filesystem_write_latency, value: be > 0, labels: { shard: :default })) } + end + + context 'storage points to directory that has both read and write rights' do + before do + FileUtils.chmod_R(0755, tmp_dir) + end + + it { is_expected.to include(metric_class.new(:filesystem_accessible, 1, shard: :default)) } + it { is_expected.to include(metric_class.new(:filesystem_readable, 1, shard: :default)) } + it { is_expected.to include(metric_class.new(:filesystem_writable, 1, shard: :default)) } + + it { is_expected.to include(have_attributes(name: :filesystem_access_latency, value: be > 0, labels: { shard: :default })) } + it { is_expected.to include(have_attributes(name: :filesystem_read_latency, value: be > 0, labels: { shard: :default })) } + it { is_expected.to include(have_attributes(name: :filesystem_write_latency, value: be > 0, labels: { shard: :default })) } + end + end + end + + context 'when popen always finds required binaries' do + before do + allow(Gitlab::Popen).to receive(:popen).and_wrap_original do |method, *args, &block| + begin + method.call(*args, &block) + rescue RuntimeError + raise 'expected not to happen' + end + end + end + + it_behaves_like 'filesystem checks' + end + + context 'when popen never finds required binaries' do + before do + allow(Gitlab::Popen).to receive(:popen).and_raise(Errno::ENOENT) + end + + it_behaves_like 'filesystem checks' + end +end diff --git a/spec/lib/gitlab/health_checks/redis_check_spec.rb b/spec/lib/gitlab/health_checks/redis_check_spec.rb new file mode 100644 index 00000000000..734cdcb893e --- /dev/null +++ b/spec/lib/gitlab/health_checks/redis_check_spec.rb @@ -0,0 +1,6 @@ +require 'spec_helper' +require_relative './simple_check_shared' + +describe Gitlab::HealthChecks::RedisCheck do + include_examples 'simple_check', 'redis_ping', 'Redis', 'PONG' +end diff --git a/spec/lib/gitlab/health_checks/simple_check_shared.rb b/spec/lib/gitlab/health_checks/simple_check_shared.rb new file mode 100644 index 00000000000..1fa6d0faef9 --- /dev/null +++ b/spec/lib/gitlab/health_checks/simple_check_shared.rb @@ -0,0 +1,66 @@ +shared_context 'simple_check' do |metrics_prefix, check_name, success_result| + describe '#metrics' do + subject { described_class.metrics } + context 'Check is passing' do + before do + allow(described_class).to receive(:check).and_return success_result + end + + it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_success", value: 1)) } + it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_timeout", value: 0)) } + it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency", value: be > 0)) } + end + + context 'Check is misbehaving' do + before do + allow(described_class).to receive(:check).and_return 'error!' + end + + it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_success", value: 0)) } + it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_timeout", value: 0)) } + it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency", value: be > 0)) } + end + + context 'Check is timeouting' do + before do + allow(described_class).to receive(:check).and_return Timeout::Error.new + end + + it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_success", value: 0)) } + it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_timeout", value: 1)) } + it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency", value: be > 0)) } + end + end + + describe '#readiness' do + subject { described_class.readiness } + context 'Check returns ok' do + before do + allow(described_class).to receive(:check).and_return success_result + end + + it { is_expected.to have_attributes(success: true) } + end + + context 'Check is misbehaving' do + before do + allow(described_class).to receive(:check).and_return 'error!' + end + + it { is_expected.to have_attributes(success: false, message: "unexpected #{check_name} check result: error!") } + end + + context 'Check is timeouting' do + before do + allow(described_class).to receive(:check ).and_return Timeout::Error.new + end + + it { is_expected.to have_attributes(success: false, message: "#{check_name} check timed out") } + end + end + + describe '#liveness' do + subject { described_class.readiness } + it { is_expected.to eq(Gitlab::HealthChecks::Result.new(true)) } + end +end diff --git a/spec/lib/gitlab/healthchecks/db_check_spec.rb b/spec/lib/gitlab/healthchecks/db_check_spec.rb deleted file mode 100644 index 33c6c24449c..00000000000 --- a/spec/lib/gitlab/healthchecks/db_check_spec.rb +++ /dev/null @@ -1,6 +0,0 @@ -require 'spec_helper' -require_relative './simple_check_shared' - -describe Gitlab::HealthChecks::DbCheck do - include_examples 'simple_check', 'db_ping', 'Db', '1' -end diff --git a/spec/lib/gitlab/healthchecks/fs_shards_check_spec.rb b/spec/lib/gitlab/healthchecks/fs_shards_check_spec.rb deleted file mode 100644 index 4cd8cf313a5..00000000000 --- a/spec/lib/gitlab/healthchecks/fs_shards_check_spec.rb +++ /dev/null @@ -1,127 +0,0 @@ -require 'spec_helper' - -describe Gitlab::HealthChecks::FsShardsCheck do - let(:metric_class) { Gitlab::HealthChecks::Metric } - let(:result_class) { Gitlab::HealthChecks::Result } - let(:repository_storages) { [:default] } - let(:tmp_dir) { Dir.mktmpdir } - - let(:storages_paths) do - { - default: { path: tmp_dir } - }.with_indifferent_access - end - - before do - allow(described_class).to receive(:repository_storages) { repository_storages } - allow(described_class).to receive(:storages_paths) { storages_paths } - end - - after do - FileUtils.remove_entry_secure(tmp_dir) if Dir.exist?(tmp_dir) - end - - shared_examples 'filesystem checks' do - describe '#readiness' do - subject { described_class.readiness } - - context 'storage points to not existing folder' do - let(:storages_paths) do - { - default: { path: 'tmp/this/path/doesnt/exist' } - }.with_indifferent_access - end - - it { is_expected.to include(result_class.new(false, 'cannot stat storage', shard: :default)) } - end - - context 'storage points to directory that has both read and write rights' do - before do - FileUtils.chmod_R(0755, tmp_dir) - end - - it { is_expected.to include(result_class.new(true, nil, shard: :default)) } - - it 'cleans up files used for testing' do - expect(described_class).to receive(:storage_write_test).with(any_args).and_call_original - - subject - - expect(Dir.entries(tmp_dir).count).to eq(2) - end - - context 'read test fails' do - before do - allow(described_class).to receive(:storage_read_test).with(any_args).and_return(false) - end - - it { is_expected.to include(result_class.new(false, 'cannot read from storage', shard: :default)) } - end - - context 'write test fails' do - before do - allow(described_class).to receive(:storage_write_test).with(any_args).and_return(false) - end - - it { is_expected.to include(result_class.new(false, 'cannot write to storage', shard: :default)) } - end - end - end - - describe '#metrics' do - subject { described_class.metrics } - - context 'storage points to not existing folder' do - let(:storages_paths) do - { - default: { path: 'tmp/this/path/doesnt/exist' } - }.with_indifferent_access - end - - it { is_expected.to include(metric_class.new(:filesystem_accessible, 0, shard: :default)) } - it { is_expected.to include(metric_class.new(:filesystem_readable, 0, shard: :default)) } - it { is_expected.to include(metric_class.new(:filesystem_writable, 0, shard: :default)) } - - it { is_expected.to include(have_attributes(name: :filesystem_access_latency, value: be > 0, labels: { shard: :default })) } - it { is_expected.to include(have_attributes(name: :filesystem_read_latency, value: be > 0, labels: { shard: :default })) } - it { is_expected.to include(have_attributes(name: :filesystem_write_latency, value: be > 0, labels: { shard: :default })) } - end - - context 'storage points to directory that has both read and write rights' do - before do - FileUtils.chmod_R(0755, tmp_dir) - end - - it { is_expected.to include(metric_class.new(:filesystem_accessible, 1, shard: :default)) } - it { is_expected.to include(metric_class.new(:filesystem_readable, 1, shard: :default)) } - it { is_expected.to include(metric_class.new(:filesystem_writable, 1, shard: :default)) } - - it { is_expected.to include(have_attributes(name: :filesystem_access_latency, value: be > 0, labels: { shard: :default })) } - it { is_expected.to include(have_attributes(name: :filesystem_read_latency, value: be > 0, labels: { shard: :default })) } - it { is_expected.to include(have_attributes(name: :filesystem_write_latency, value: be > 0, labels: { shard: :default })) } - end - end - end - - context 'when popen always finds required binaries' do - before do - allow(Gitlab::Popen).to receive(:popen).and_wrap_original do |method, *args, &block| - begin - method.call(*args, &block) - rescue RuntimeError - raise 'expected not to happen' - end - end - end - - it_behaves_like 'filesystem checks' - end - - context 'when popen never finds required binaries' do - before do - allow(Gitlab::Popen).to receive(:popen).and_raise(Errno::ENOENT) - end - - it_behaves_like 'filesystem checks' - end -end diff --git a/spec/lib/gitlab/healthchecks/redis_check_spec.rb b/spec/lib/gitlab/healthchecks/redis_check_spec.rb deleted file mode 100644 index 734cdcb893e..00000000000 --- a/spec/lib/gitlab/healthchecks/redis_check_spec.rb +++ /dev/null @@ -1,6 +0,0 @@ -require 'spec_helper' -require_relative './simple_check_shared' - -describe Gitlab::HealthChecks::RedisCheck do - include_examples 'simple_check', 'redis_ping', 'Redis', 'PONG' -end diff --git a/spec/lib/gitlab/healthchecks/simple_check_shared.rb b/spec/lib/gitlab/healthchecks/simple_check_shared.rb deleted file mode 100644 index 1fa6d0faef9..00000000000 --- a/spec/lib/gitlab/healthchecks/simple_check_shared.rb +++ /dev/null @@ -1,66 +0,0 @@ -shared_context 'simple_check' do |metrics_prefix, check_name, success_result| - describe '#metrics' do - subject { described_class.metrics } - context 'Check is passing' do - before do - allow(described_class).to receive(:check).and_return success_result - end - - it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_success", value: 1)) } - it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_timeout", value: 0)) } - it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency", value: be > 0)) } - end - - context 'Check is misbehaving' do - before do - allow(described_class).to receive(:check).and_return 'error!' - end - - it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_success", value: 0)) } - it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_timeout", value: 0)) } - it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency", value: be > 0)) } - end - - context 'Check is timeouting' do - before do - allow(described_class).to receive(:check).and_return Timeout::Error.new - end - - it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_success", value: 0)) } - it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_timeout", value: 1)) } - it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency", value: be > 0)) } - end - end - - describe '#readiness' do - subject { described_class.readiness } - context 'Check returns ok' do - before do - allow(described_class).to receive(:check).and_return success_result - end - - it { is_expected.to have_attributes(success: true) } - end - - context 'Check is misbehaving' do - before do - allow(described_class).to receive(:check).and_return 'error!' - end - - it { is_expected.to have_attributes(success: false, message: "unexpected #{check_name} check result: error!") } - end - - context 'Check is timeouting' do - before do - allow(described_class).to receive(:check ).and_return Timeout::Error.new - end - - it { is_expected.to have_attributes(success: false, message: "#{check_name} check timed out") } - end - end - - describe '#liveness' do - subject { described_class.readiness } - it { is_expected.to eq(Gitlab::HealthChecks::Result.new(true)) } - end -end diff --git a/spec/lib/gitlab/import_export/wiki_repo_bundler_spec.rb b/spec/lib/gitlab/import_export/wiki_repo_bundler_spec.rb deleted file mode 100644 index 071e5fac3f0..00000000000 --- a/spec/lib/gitlab/import_export/wiki_repo_bundler_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -require 'spec_helper' - -describe Gitlab::ImportExport::WikiRepoSaver, services: true do - describe 'bundle a wiki Git repo' do - let(:user) { create(:user) } - let!(:project) { create(:empty_project, :public, name: 'searchable_project') } - let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } - let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.path_with_namespace) } - let(:wiki_bundler) { described_class.new(project: project, shared: shared) } - let!(:project_wiki) { ProjectWiki.new(project, user) } - - before do - project.team << [user, :master] - allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) - project_wiki.wiki - project_wiki.create_page("index", "test content") - end - - after do - FileUtils.rm_rf(export_path) - end - - it 'bundles the repo successfully' do - expect(wiki_bundler.save).to be true - end - end -end diff --git a/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb b/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb new file mode 100644 index 00000000000..071e5fac3f0 --- /dev/null +++ b/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe Gitlab::ImportExport::WikiRepoSaver, services: true do + describe 'bundle a wiki Git repo' do + let(:user) { create(:user) } + let!(:project) { create(:empty_project, :public, name: 'searchable_project') } + let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } + let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.path_with_namespace) } + let(:wiki_bundler) { described_class.new(project: project, shared: shared) } + let!(:project_wiki) { ProjectWiki.new(project, user) } + + before do + project.team << [user, :master] + allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + project_wiki.wiki + project_wiki.create_page("index", "test content") + end + + after do + FileUtils.rm_rf(export_path) + end + + it 'bundles the repo successfully' do + expect(wiki_bundler.save).to be true + end + end +end diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb new file mode 100644 index 00000000000..6675d26734e --- /dev/null +++ b/spec/lib/gitlab/shell_spec.rb @@ -0,0 +1,94 @@ +require 'spec_helper' +require 'stringio' + +describe Gitlab::Shell, lib: true do + let(:project) { double('Project', id: 7, path: 'diaspora') } + let(:gitlab_shell) { Gitlab::Shell.new } + + before do + allow(Project).to receive(:find).and_return(project) + end + + it { is_expected.to respond_to :add_key } + it { is_expected.to respond_to :remove_key } + it { is_expected.to respond_to :add_repository } + it { is_expected.to respond_to :remove_repository } + it { is_expected.to respond_to :fork_repository } + it { is_expected.to respond_to :add_namespace } + it { is_expected.to respond_to :rm_namespace } + it { is_expected.to respond_to :mv_namespace } + it { is_expected.to respond_to :exists? } + + it { expect(gitlab_shell.url_to_repo('diaspora')).to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + "diaspora.git") } + + describe 'memoized secret_token' do + let(:secret_file) { 'tmp/tests/.secret_shell_test' } + let(:link_file) { 'tmp/tests/shell-secret-test/.gitlab_shell_secret' } + + before do + allow(Gitlab.config.gitlab_shell).to receive(:secret_file).and_return(secret_file) + allow(Gitlab.config.gitlab_shell).to receive(:path).and_return('tmp/tests/shell-secret-test') + FileUtils.mkdir('tmp/tests/shell-secret-test') + Gitlab::Shell.ensure_secret_token! + end + + after do + FileUtils.rm_rf('tmp/tests/shell-secret-test') + FileUtils.rm_rf(secret_file) + end + + it 'creates and links the secret token file' do + secret_token = Gitlab::Shell.secret_token + + expect(File.exist?(secret_file)).to be(true) + expect(File.read(secret_file).chomp).to eq(secret_token) + expect(File.symlink?(link_file)).to be(true) + expect(File.readlink(link_file)).to eq(secret_file) + end + end + + describe '#add_key' do + it 'removes trailing garbage' do + allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) + expect(Gitlab::Utils).to receive(:system_silent).with( + [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar'] + ) + + gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') + end + end + + describe Gitlab::Shell::KeyAdder, lib: true do + describe '#add_key' do + it 'removes trailing garbage' do + io = spy(:io) + adder = described_class.new(io) + + adder.add_key('key-42', "ssh-rsa foo bar\tbaz") + + expect(io).to have_received(:puts).with("key-42\tssh-rsa foo") + end + + it 'handles multiple spaces in the key' do + io = spy(:io) + adder = described_class.new(io) + + adder.add_key('key-42', "ssh-rsa foo") + + expect(io).to have_received(:puts).with("key-42\tssh-rsa foo") + end + + it 'raises an exception if the key contains a tab' do + expect do + described_class.new(StringIO.new).add_key('key-42', "ssh-rsa\tfoobar") + end.to raise_error(Gitlab::Shell::Error) + end + + it 'raises an exception if the key contains a newline' do + expect do + described_class.new(StringIO.new).add_key('key-42', "ssh-rsa foobar\nssh-rsa pawned") + end.to raise_error(Gitlab::Shell::Error) + end + end + end +end diff --git a/spec/lib/light_url_builder_spec.rb b/spec/lib/light_url_builder_spec.rb deleted file mode 100644 index 3fe8cf43934..00000000000 --- a/spec/lib/light_url_builder_spec.rb +++ /dev/null @@ -1,119 +0,0 @@ -require 'spec_helper' - -describe Gitlab::UrlBuilder, lib: true do - describe '.build' do - context 'when passing a Commit' do - it 'returns a proper URL' do - commit = build_stubbed(:commit) - - url = described_class.build(commit) - - expect(url).to eq "#{Settings.gitlab['url']}/#{commit.project.path_with_namespace}/commit/#{commit.id}" - end - end - - context 'when passing an Issue' do - it 'returns a proper URL' do - issue = build_stubbed(:issue, iid: 42) - - url = described_class.build(issue) - - expect(url).to eq "#{Settings.gitlab['url']}/#{issue.project.path_with_namespace}/issues/#{issue.iid}" - end - end - - context 'when passing a MergeRequest' do - it 'returns a proper URL' do - merge_request = build_stubbed(:merge_request, iid: 42) - - url = described_class.build(merge_request) - - expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}" - end - end - - context 'when passing a Note' do - context 'on a Commit' do - it 'returns a proper URL' do - note = build_stubbed(:note_on_commit) - - url = described_class.build(note) - - expect(url).to eq "#{Settings.gitlab['url']}/#{note.project.path_with_namespace}/commit/#{note.commit_id}#note_#{note.id}" - end - end - - context 'on a Commit Diff' do - it 'returns a proper URL' do - note = build_stubbed(:diff_note_on_commit) - - url = described_class.build(note) - - expect(url).to eq "#{Settings.gitlab['url']}/#{note.project.path_with_namespace}/commit/#{note.commit_id}#note_#{note.id}" - end - end - - context 'on an Issue' do - it 'returns a proper URL' do - issue = create(:issue, iid: 42) - note = build_stubbed(:note_on_issue, noteable: issue) - - url = described_class.build(note) - - expect(url).to eq "#{Settings.gitlab['url']}/#{issue.project.path_with_namespace}/issues/#{issue.iid}#note_#{note.id}" - end - end - - context 'on a MergeRequest' do - it 'returns a proper URL' do - merge_request = create(:merge_request, iid: 42) - note = build_stubbed(:note_on_merge_request, noteable: merge_request) - - url = described_class.build(note) - - expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}#note_#{note.id}" - end - end - - context 'on a MergeRequest Diff' do - it 'returns a proper URL' do - merge_request = create(:merge_request, iid: 42) - note = build_stubbed(:diff_note_on_merge_request, noteable: merge_request) - - url = described_class.build(note) - - expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}#note_#{note.id}" - end - end - - context 'on a ProjectSnippet' do - it 'returns a proper URL' do - project_snippet = create(:project_snippet) - note = build_stubbed(:note_on_project_snippet, noteable: project_snippet) - - url = described_class.build(note) - - expect(url).to eq "#{Settings.gitlab['url']}/#{project_snippet.project.path_with_namespace}/snippets/#{note.noteable_id}#note_#{note.id}" - end - end - - context 'on another object' do - it 'returns a proper URL' do - project = build_stubbed(:empty_project) - - expect { described_class.build(project) }. - to raise_error(NotImplementedError, 'No URL builder defined for Project') - end - end - end - - context 'when passing a WikiPage' do - it 'returns a proper URL' do - wiki_page = build(:wiki_page) - url = described_class.build(wiki_page) - - expect(url).to eq "#{Gitlab.config.gitlab.url}#{wiki_page.wiki.wiki_base_path}/#{wiki_page.slug}" - end - end - end -end diff --git a/spec/mailers/emails/merge_requests_spec.rb b/spec/mailers/emails/merge_requests_spec.rb index e22858d1d8f..2ad572bb5c7 100644 --- a/spec/mailers/emails/merge_requests_spec.rb +++ b/spec/mailers/emails/merge_requests_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' require 'email_spec' -describe Notify, "merge request notifications" do +describe Emails::MergeRequests do include EmailSpec::Matchers describe "#resolved_all_discussions_email" do diff --git a/spec/mailers/emails/profile_spec.rb b/spec/mailers/emails/profile_spec.rb index 5ca936f28f0..8c1c9bf135f 100644 --- a/spec/mailers/emails/profile_spec.rb +++ b/spec/mailers/emails/profile_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' require 'email_spec' -describe Notify do +describe Emails::Profile do include EmailSpec::Matchers include_context 'gitlab email notification' @@ -15,106 +15,104 @@ describe Notify do end end - describe 'profile notifications' do - describe 'for new users, the email' do - let(:example_site_path) { root_path } - let(:new_user) { create(:user, email: new_user_address, created_by_id: 1) } - let(:token) { 'kETLwRaayvigPq_x3SNM' } + describe 'for new users, the email' do + let(:example_site_path) { root_path } + let(:new_user) { create(:user, email: new_user_address, created_by_id: 1) } + let(:token) { 'kETLwRaayvigPq_x3SNM' } - subject { Notify.new_user_email(new_user.id, token) } + subject { Notify.new_user_email(new_user.id, token) } - it_behaves_like 'an email sent from GitLab' - it_behaves_like 'a new user email' - it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like 'a user cannot unsubscribe through footer link' + it_behaves_like 'an email sent from GitLab' + it_behaves_like 'a new user email' + it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like 'a user cannot unsubscribe through footer link' - it 'contains the password text' do - is_expected.to have_body_text /Click here to set your password/ - end + it 'contains the password text' do + is_expected.to have_body_text /Click here to set your password/ + end - it 'includes a link for user to set password' do - params = "reset_password_token=#{token}" - is_expected.to have_body_text( - %r{http://#{Gitlab.config.gitlab.host}(:\d+)?/users/password/edit\?#{params}} - ) - end + it 'includes a link for user to set password' do + params = "reset_password_token=#{token}" + is_expected.to have_body_text( + %r{http://#{Gitlab.config.gitlab.host}(:\d+)?/users/password/edit\?#{params}} + ) + end - it 'explains the reset link expiration' do - is_expected.to have_body_text(/This link is valid for \d+ (hours?|days?)/) - is_expected.to have_body_text(new_user_password_url) - is_expected.to have_body_text(/\?user_email=.*%40.*/) - end + it 'explains the reset link expiration' do + is_expected.to have_body_text(/This link is valid for \d+ (hours?|days?)/) + is_expected.to have_body_text(new_user_password_url) + is_expected.to have_body_text(/\?user_email=.*%40.*/) end + end - describe 'for users that signed up, the email' do - let(:example_site_path) { root_path } - let(:new_user) { create(:user, email: new_user_address, password: "securePassword") } + describe 'for users that signed up, the email' do + let(:example_site_path) { root_path } + let(:new_user) { create(:user, email: new_user_address, password: "securePassword") } - subject { Notify.new_user_email(new_user.id) } + subject { Notify.new_user_email(new_user.id) } - it_behaves_like 'an email sent from GitLab' - it_behaves_like 'a new user email' - it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like 'a user cannot unsubscribe through footer link' + it_behaves_like 'an email sent from GitLab' + it_behaves_like 'a new user email' + it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like 'a user cannot unsubscribe through footer link' - it 'does not contain the new user\'s password' do - is_expected.not_to have_body_text /password/ - end + it 'does not contain the new user\'s password' do + is_expected.not_to have_body_text /password/ end + end - describe 'user added ssh key' do - let(:key) { create(:personal_key) } + describe 'user added ssh key' do + let(:key) { create(:personal_key) } - subject { Notify.new_ssh_key_email(key.id) } + subject { Notify.new_ssh_key_email(key.id) } - it_behaves_like 'an email sent from GitLab' - it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like 'a user cannot unsubscribe through footer link' + it_behaves_like 'an email sent from GitLab' + it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like 'a user cannot unsubscribe through footer link' - it 'is sent to the new user' do - is_expected.to deliver_to key.user.email - end + it 'is sent to the new user' do + is_expected.to deliver_to key.user.email + end - it 'has the correct subject' do - is_expected.to have_subject /^SSH key was added to your account$/i - end + it 'has the correct subject' do + is_expected.to have_subject /^SSH key was added to your account$/i + end - it 'contains the new ssh key title' do - is_expected.to have_body_text /#{key.title}/ - end + it 'contains the new ssh key title' do + is_expected.to have_body_text /#{key.title}/ + end - it 'includes a link to ssh keys page' do - is_expected.to have_body_text /#{profile_keys_path}/ - end + it 'includes a link to ssh keys page' do + is_expected.to have_body_text /#{profile_keys_path}/ + end - context 'with SSH key that does not exist' do - it { expect { Notify.new_ssh_key_email('foo') }.not_to raise_error } - end + context 'with SSH key that does not exist' do + it { expect { Notify.new_ssh_key_email('foo') }.not_to raise_error } end + end - describe 'user added email' do - let(:email) { create(:email) } + describe 'user added email' do + let(:email) { create(:email) } - subject { Notify.new_email_email(email.id) } + subject { Notify.new_email_email(email.id) } - it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like 'a user cannot unsubscribe through footer link' + it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like 'a user cannot unsubscribe through footer link' - it 'is sent to the new user' do - is_expected.to deliver_to email.user.email - end + it 'is sent to the new user' do + is_expected.to deliver_to email.user.email + end - it 'has the correct subject' do - is_expected.to have_subject /^Email was added to your account$/i - end + it 'has the correct subject' do + is_expected.to have_subject /^Email was added to your account$/i + end - it 'contains the new email address' do - is_expected.to have_body_text /#{email.email}/ - end + it 'contains the new email address' do + is_expected.to have_body_text /#{email.email}/ + end - it 'includes a link to emails page' do - is_expected.to have_body_text /#{profile_emails_path}/ - end + it 'includes a link to emails page' do + is_expected.to have_body_text /#{profile_emails_path}/ end end end diff --git a/spec/migrations/active_record/schema_spec.rb b/spec/migrations/active_record/schema_spec.rb new file mode 100644 index 00000000000..e132529d8d8 --- /dev/null +++ b/spec/migrations/active_record/schema_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +# Check consistency of db/schema.rb version, migrations' timestamps, and the latest migration timestamp +# stored in the database's schema_migrations table. + +describe ActiveRecord::Schema do + let(:latest_migration_timestamp) do + migrations = Dir[Rails.root.join('db', 'migrate', '*'), Rails.root.join('db', 'post_migrate', '*')] + migrations.map { |migration| File.basename(migration).split('_').first.to_i }.max + end + + it '> schema version equals last migration timestamp' do + defined_schema_version = File.open(Rails.root.join('db', 'schema.rb')) do |file| + file.find { |line| line =~ /ActiveRecord::Schema.define/ } + end.match(/(\d+)/)[0].to_i + + expect(defined_schema_version).to eq(latest_migration_timestamp) + end + + it '> schema version should equal the latest migration timestamp stored in schema_migrations table' do + expect(latest_migration_timestamp).to eq(ActiveRecord::Migrator.current_version.to_i) + end +end diff --git a/spec/migrations/schema_spec.rb b/spec/migrations/schema_spec.rb deleted file mode 100644 index e132529d8d8..00000000000 --- a/spec/migrations/schema_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'spec_helper' - -# Check consistency of db/schema.rb version, migrations' timestamps, and the latest migration timestamp -# stored in the database's schema_migrations table. - -describe ActiveRecord::Schema do - let(:latest_migration_timestamp) do - migrations = Dir[Rails.root.join('db', 'migrate', '*'), Rails.root.join('db', 'post_migrate', '*')] - migrations.map { |migration| File.basename(migration).split('_').first.to_i }.max - end - - it '> schema version equals last migration timestamp' do - defined_schema_version = File.open(Rails.root.join('db', 'schema.rb')) do |file| - file.find { |line| line =~ /ActiveRecord::Schema.define/ } - end.match(/(\d+)/)[0].to_i - - expect(defined_schema_version).to eq(latest_migration_timestamp) - end - - it '> schema version should equal the latest migration timestamp stored in schema_migrations table' do - expect(latest_migration_timestamp).to eq(ActiveRecord::Migrator.current_version.to_i) - end -end diff --git a/spec/models/concerns/awardable_spec.rb b/spec/models/concerns/awardable_spec.rb index de791abdf3d..63ad3a3630b 100644 --- a/spec/models/concerns/awardable_spec.rb +++ b/spec/models/concerns/awardable_spec.rb @@ -1,10 +1,12 @@ require 'spec_helper' -describe Issue, "Awardable" do +describe Awardable do let!(:issue) { create(:issue) } let!(:award_emoji) { create(:award_emoji, :downvote, awardable: issue) } describe "Associations" do + subject { build(:issue) } + it { is_expected.to have_many(:award_emoji).dependent(:destroy) } end diff --git a/spec/models/concerns/discussion_on_diff_spec.rb b/spec/models/concerns/discussion_on_diff_spec.rb index 0002a00770f..8571e85627c 100644 --- a/spec/models/concerns/discussion_on_diff_spec.rb +++ b/spec/models/concerns/discussion_on_diff_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe DiffDiscussion, DiscussionOnDiff, model: true do +describe DiscussionOnDiff, model: true do subject { create(:diff_note_on_merge_request).to_discussion } describe "#truncated_diff_lines" do @@ -8,9 +8,9 @@ describe DiffDiscussion, DiscussionOnDiff, model: true do context "when diff is greater than allowed number of truncated diff lines " do it "returns fewer lines" do - expect(subject.diff_lines.count).to be > described_class::NUMBER_OF_TRUNCATED_DIFF_LINES + expect(subject.diff_lines.count).to be > DiffDiscussion::NUMBER_OF_TRUNCATED_DIFF_LINES - expect(truncated_lines.count).to be <= described_class::NUMBER_OF_TRUNCATED_DIFF_LINES + expect(truncated_lines.count).to be <= DiffDiscussion::NUMBER_OF_TRUNCATED_DIFF_LINES end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 4522206fab1..3ecba2e9687 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -1,10 +1,13 @@ require 'spec_helper' -describe Issue, "Issuable" do +describe Issuable do + let(:issuable_class) { Issue } let(:issue) { create(:issue) } let(:user) { create(:user) } describe "Associations" do + subject { build(:issue) } + it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:author) } it { is_expected.to belong_to(:assignee) } @@ -23,10 +26,14 @@ describe Issue, "Issuable" do end describe 'Included modules' do + let(:described_class) { issuable_class } + it { is_expected.to include_module(Awardable) } end describe "Validation" do + subject { build(:issue) } + before do allow(subject).to receive(:set_iid).and_return(false) end @@ -39,9 +46,11 @@ describe Issue, "Issuable" do end describe "Scope" do - it { expect(described_class).to respond_to(:opened) } - it { expect(described_class).to respond_to(:closed) } - it { expect(described_class).to respond_to(:assigned) } + subject { build(:issue) } + + it { expect(issuable_class).to respond_to(:opened) } + it { expect(issuable_class).to respond_to(:closed) } + it { expect(issuable_class).to respond_to(:assigned) } end describe 'author_name' do @@ -115,16 +124,16 @@ describe Issue, "Issuable" do let!(:searchable_issue) { create(:issue, title: "Searchable issue") } it 'returns notes with a matching title' do - expect(described_class.search(searchable_issue.title)). + expect(issuable_class.search(searchable_issue.title)). to eq([searchable_issue]) end it 'returns notes with a partially matching title' do - expect(described_class.search('able')).to eq([searchable_issue]) + expect(issuable_class.search('able')).to eq([searchable_issue]) end it 'returns notes with a matching title regardless of the casing' do - expect(described_class.search(searchable_issue.title.upcase)). + expect(issuable_class.search(searchable_issue.title.upcase)). to eq([searchable_issue]) end end @@ -135,31 +144,31 @@ describe Issue, "Issuable" do end it 'returns notes with a matching title' do - expect(described_class.full_search(searchable_issue.title)). + expect(issuable_class.full_search(searchable_issue.title)). to eq([searchable_issue]) end it 'returns notes with a partially matching title' do - expect(described_class.full_search('able')).to eq([searchable_issue]) + expect(issuable_class.full_search('able')).to eq([searchable_issue]) end it 'returns notes with a matching title regardless of the casing' do - expect(described_class.full_search(searchable_issue.title.upcase)). + expect(issuable_class.full_search(searchable_issue.title.upcase)). to eq([searchable_issue]) end it 'returns notes with a matching description' do - expect(described_class.full_search(searchable_issue.description)). + expect(issuable_class.full_search(searchable_issue.description)). to eq([searchable_issue]) end it 'returns notes with a partially matching description' do - expect(described_class.full_search(searchable_issue.description)). + expect(issuable_class.full_search(searchable_issue.description)). to eq([searchable_issue]) end it 'returns notes with a matching description regardless of the casing' do - expect(described_class.full_search(searchable_issue.description.upcase)). + expect(issuable_class.full_search(searchable_issue.description.upcase)). to eq([searchable_issue]) end end diff --git a/spec/models/concerns/noteable_spec.rb b/spec/models/concerns/noteable_spec.rb index 92cc8859a8c..bdae742ff1d 100644 --- a/spec/models/concerns/noteable_spec.rb +++ b/spec/models/concerns/noteable_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe MergeRequest, Noteable, model: true do +describe Noteable, model: true do let!(:active_diff_note1) { create(:diff_note_on_merge_request) } let(:project) { active_diff_note1.project } subject { active_diff_note1.noteable } diff --git a/spec/models/concerns/relative_positioning_spec.rb b/spec/models/concerns/relative_positioning_spec.rb index 255b584a85e..494e6f1b6f6 100644 --- a/spec/models/concerns/relative_positioning_spec.rb +++ b/spec/models/concerns/relative_positioning_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Issue, 'RelativePositioning' do +describe RelativePositioning do let(:project) { create(:empty_project) } let(:issue) { create(:issue, project: project) } let(:issue1) { create(:issue, project: project) } diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb index fd3b8307571..e698207166c 100644 --- a/spec/models/concerns/spammable_spec.rb +++ b/spec/models/concerns/spammable_spec.rb @@ -1,9 +1,11 @@ require 'spec_helper' -describe Issue, 'Spammable' do +describe Spammable do let(:issue) { create(:issue, description: 'Test Desc.') } describe 'Associations' do + subject { build(:issue) } + it { is_expected.to have_one(:user_agent_detail).dependent(:destroy) } end diff --git a/spec/models/concerns/strip_attribute_spec.rb b/spec/models/concerns/strip_attribute_spec.rb index c3af7a0960f..8c945686b66 100644 --- a/spec/models/concerns/strip_attribute_spec.rb +++ b/spec/models/concerns/strip_attribute_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Milestone, "StripAttribute" do +describe StripAttribute do let(:milestone) { create(:milestone) } describe ".strip_attributes" do diff --git a/spec/models/project_services/pipeline_email_service_spec.rb b/spec/models/project_services/pipeline_email_service_spec.rb deleted file mode 100644 index 03932895b0e..00000000000 --- a/spec/models/project_services/pipeline_email_service_spec.rb +++ /dev/null @@ -1,175 +0,0 @@ -require 'spec_helper' - -describe PipelinesEmailService do - include EmailHelpers - - let(:pipeline) do - create(:ci_pipeline, project: project, sha: project.commit('master').sha) - end - - let(:project) { create(:project, :repository) } - let(:recipient) { 'test@gitlab.com' } - - let(:data) do - Gitlab::DataBuilder::Pipeline.build(pipeline) - end - - before do - reset_delivered_emails! - end - - describe 'Validations' do - context 'when service is active' do - before do - subject.active = true - end - - it { is_expected.to validate_presence_of(:recipients) } - end - - context 'when service is inactive' do - before do - subject.active = false - end - - it { is_expected.not_to validate_presence_of(:recipients) } - end - end - - describe '#test_data' do - let(:build) { create(:ci_build) } - let(:project) { build.project } - let(:user) { create(:user) } - - before do - project.team << [user, :developer] - end - - it 'builds test data' do - data = subject.test_data(project, user) - - expect(data[:object_kind]).to eq('pipeline') - end - end - - shared_examples 'sending email' do - before do - perform_enqueued_jobs do - run - end - end - - it 'sends email' do - should_only_email(double(notification_email: recipient), kind: :bcc) - end - end - - shared_examples 'not sending email' do - before do - perform_enqueued_jobs do - run - end - end - - it 'does not send email' do - should_not_email_anyone - end - end - - describe '#test' do - def run - subject.test(data) - end - - before do - subject.recipients = recipient - end - - context 'when pipeline is failed' do - before do - data[:object_attributes][:status] = 'failed' - pipeline.update(status: 'failed') - end - - it_behaves_like 'sending email' - end - - context 'when pipeline is succeeded' do - before do - data[:object_attributes][:status] = 'success' - pipeline.update(status: 'success') - end - - it_behaves_like 'sending email' - end - end - - describe '#execute' do - def run - subject.execute(data) - end - - context 'with recipients' do - before do - subject.recipients = recipient - end - - context 'with failed pipeline' do - before do - data[:object_attributes][:status] = 'failed' - pipeline.update(status: 'failed') - end - - it_behaves_like 'sending email' - end - - context 'with succeeded pipeline' do - before do - data[:object_attributes][:status] = 'success' - pipeline.update(status: 'success') - end - - it_behaves_like 'not sending email' - end - - context 'with notify_only_broken_pipelines on' do - before do - subject.notify_only_broken_pipelines = true - end - - context 'with failed pipeline' do - before do - data[:object_attributes][:status] = 'failed' - pipeline.update(status: 'failed') - end - - it_behaves_like 'sending email' - end - - context 'with succeeded pipeline' do - before do - data[:object_attributes][:status] = 'success' - pipeline.update(status: 'success') - end - - it_behaves_like 'not sending email' - end - end - end - - context 'with empty recipients list' do - before do - subject.recipients = ' ,, ' - end - - context 'with failed pipeline' do - before do - data[:object_attributes][:status] = 'failed' - pipeline.update(status: 'failed') - end - - it_behaves_like 'not sending email' - end - end - end -end diff --git a/spec/models/project_services/pipelines_email_service_spec.rb b/spec/models/project_services/pipelines_email_service_spec.rb new file mode 100644 index 00000000000..03932895b0e --- /dev/null +++ b/spec/models/project_services/pipelines_email_service_spec.rb @@ -0,0 +1,175 @@ +require 'spec_helper' + +describe PipelinesEmailService do + include EmailHelpers + + let(:pipeline) do + create(:ci_pipeline, project: project, sha: project.commit('master').sha) + end + + let(:project) { create(:project, :repository) } + let(:recipient) { 'test@gitlab.com' } + + let(:data) do + Gitlab::DataBuilder::Pipeline.build(pipeline) + end + + before do + reset_delivered_emails! + end + + describe 'Validations' do + context 'when service is active' do + before do + subject.active = true + end + + it { is_expected.to validate_presence_of(:recipients) } + end + + context 'when service is inactive' do + before do + subject.active = false + end + + it { is_expected.not_to validate_presence_of(:recipients) } + end + end + + describe '#test_data' do + let(:build) { create(:ci_build) } + let(:project) { build.project } + let(:user) { create(:user) } + + before do + project.team << [user, :developer] + end + + it 'builds test data' do + data = subject.test_data(project, user) + + expect(data[:object_kind]).to eq('pipeline') + end + end + + shared_examples 'sending email' do + before do + perform_enqueued_jobs do + run + end + end + + it 'sends email' do + should_only_email(double(notification_email: recipient), kind: :bcc) + end + end + + shared_examples 'not sending email' do + before do + perform_enqueued_jobs do + run + end + end + + it 'does not send email' do + should_not_email_anyone + end + end + + describe '#test' do + def run + subject.test(data) + end + + before do + subject.recipients = recipient + end + + context 'when pipeline is failed' do + before do + data[:object_attributes][:status] = 'failed' + pipeline.update(status: 'failed') + end + + it_behaves_like 'sending email' + end + + context 'when pipeline is succeeded' do + before do + data[:object_attributes][:status] = 'success' + pipeline.update(status: 'success') + end + + it_behaves_like 'sending email' + end + end + + describe '#execute' do + def run + subject.execute(data) + end + + context 'with recipients' do + before do + subject.recipients = recipient + end + + context 'with failed pipeline' do + before do + data[:object_attributes][:status] = 'failed' + pipeline.update(status: 'failed') + end + + it_behaves_like 'sending email' + end + + context 'with succeeded pipeline' do + before do + data[:object_attributes][:status] = 'success' + pipeline.update(status: 'success') + end + + it_behaves_like 'not sending email' + end + + context 'with notify_only_broken_pipelines on' do + before do + subject.notify_only_broken_pipelines = true + end + + context 'with failed pipeline' do + before do + data[:object_attributes][:status] = 'failed' + pipeline.update(status: 'failed') + end + + it_behaves_like 'sending email' + end + + context 'with succeeded pipeline' do + before do + data[:object_attributes][:status] = 'success' + pipeline.update(status: 'success') + end + + it_behaves_like 'not sending email' + end + end + end + + context 'with empty recipients list' do + before do + subject.recipients = ' ,, ' + end + + context 'with failed pipeline' do + before do + data[:object_attributes][:status] = 'failed' + pipeline.update(status: 'failed') + end + + it_behaves_like 'not sending email' + end + end + end +end diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb index 2905d5b26a5..9a870b7fda1 100644 --- a/spec/policies/issue_policy_spec.rb +++ b/spec/policies/issue_policy_spec.rb @@ -1,118 +1,192 @@ require 'spec_helper' describe IssuePolicy, models: true do - let(:user) { create(:user) } - - describe '#rules' do - context 'using a regular issue' do - let(:project) { create(:empty_project, :public) } - let(:issue) { create(:issue, project: project) } - let(:policies) { described_class.abilities(user, issue).to_set } - - context 'with a regular user' do - it 'includes the read_issue permission' do - expect(policies).to include(:read_issue) - end - - it 'does not include the admin_issue permission' do - expect(policies).not_to include(:admin_issue) - end - - it 'does not include the update_issue permission' do - expect(policies).not_to include(:update_issue) - end - end + let(:guest) { create(:user) } + let(:author) { create(:user) } + let(:assignee) { create(:user) } + let(:reporter) { create(:user) } + let(:group) { create(:group, :public) } + let(:reporter_from_group_link) { create(:user) } + + def permissions(user, issue) + described_class.abilities(user, issue).to_set + end + + context 'a private project' do + let(:non_member) { create(:user) } + let(:project) { create(:empty_project, :private) } + let(:issue) { create(:issue, project: project, assignee: assignee, author: author) } + let(:issue_no_assignee) { create(:issue, project: project) } + + before do + project.team << [guest, :guest] + project.team << [author, :guest] + project.team << [assignee, :guest] + project.team << [reporter, :reporter] + + group.add_reporter(reporter_from_group_link) + + create(:project_group_link, group: group, project: project) + end + + it 'does not allow non-members to read issues' do + expect(permissions(non_member, issue)).not_to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(non_member, issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows guests to read issues' do + expect(permissions(guest, issue)).to include(:read_issue) + expect(permissions(guest, issue)).not_to include(:update_issue, :admin_issue) + + expect(permissions(guest, issue_no_assignee)).to include(:read_issue) + expect(permissions(guest, issue_no_assignee)).not_to include(:update_issue, :admin_issue) + end + + it 'allows reporters to read, update, and admin issues' do + expect(permissions(reporter, issue)).to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter, issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows reporters from group links to read, update, and admin issues' do + expect(permissions(reporter_from_group_link, issue)).to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter_from_group_link, issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows issue authors to read and update their issues' do + expect(permissions(author, issue)).to include(:read_issue, :update_issue) + expect(permissions(author, issue)).not_to include(:admin_issue) + + expect(permissions(author, issue_no_assignee)).to include(:read_issue) + expect(permissions(author, issue_no_assignee)).not_to include(:update_issue, :admin_issue) + end + + it 'allows issue assignees to read and update their issues' do + expect(permissions(assignee, issue)).to include(:read_issue, :update_issue) + expect(permissions(assignee, issue)).not_to include(:admin_issue) + + expect(permissions(assignee, issue_no_assignee)).to include(:read_issue) + expect(permissions(assignee, issue_no_assignee)).not_to include(:update_issue, :admin_issue) + end - context 'with a user that is a project reporter' do - before do - project.team << [user, :reporter] - end + context 'with confidential issues' do + let(:confidential_issue) { create(:issue, :confidential, project: project, assignee: assignee, author: author) } + let(:confidential_issue_no_assignee) { create(:issue, :confidential, project: project) } - it 'includes the read_issue permission' do - expect(policies).to include(:read_issue) - end + it 'does not allow non-members to read confidential issues' do + expect(permissions(non_member, confidential_issue)).not_to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(non_member, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) + end + + it 'does not allow guests to read confidential issues' do + expect(permissions(guest, confidential_issue)).not_to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(guest, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) + end - it 'includes the admin_issue permission' do - expect(policies).to include(:admin_issue) - end + it 'allows reporters to read, update, and admin confidential issues' do + expect(permissions(reporter, confidential_issue)).to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter, confidential_issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) + end - it 'includes the update_issue permission' do - expect(policies).to include(:update_issue) - end + it 'allows reporters from group links to read, update, and admin confidential issues' do + expect(permissions(reporter_from_group_link, confidential_issue)).to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter_from_group_link, confidential_issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) end - context 'with a user that is a project guest' do - before do - project.team << [user, :guest] - end + it 'allows issue authors to read and update their confidential issues' do + expect(permissions(author, confidential_issue)).to include(:read_issue, :update_issue) + expect(permissions(author, confidential_issue)).not_to include(:admin_issue) - it 'includes the read_issue permission' do - expect(policies).to include(:read_issue) - end + expect(permissions(author, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) + end - it 'does not include the admin_issue permission' do - expect(policies).not_to include(:admin_issue) - end + it 'allows issue assignees to read and update their confidential issues' do + expect(permissions(assignee, confidential_issue)).to include(:read_issue, :update_issue) + expect(permissions(assignee, confidential_issue)).not_to include(:admin_issue) - it 'does not include the update_issue permission' do - expect(policies).not_to include(:update_issue) - end + expect(permissions(assignee, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) end end + end - context 'using a confidential issue' do - let(:issue) { create(:issue, :confidential) } + context 'a public project' do + let(:project) { create(:empty_project, :public) } + let(:issue) { create(:issue, project: project, assignee: assignee, author: author) } + let(:issue_no_assignee) { create(:issue, project: project) } - context 'with a regular user' do - let(:policies) { described_class.abilities(user, issue).to_set } + before do + project.team << [guest, :guest] + project.team << [reporter, :reporter] - it 'does not include the read_issue permission' do - expect(policies).not_to include(:read_issue) - end + group.add_reporter(reporter_from_group_link) - it 'does not include the admin_issue permission' do - expect(policies).not_to include(:admin_issue) - end + create(:project_group_link, group: group, project: project) + end - it 'does not include the update_issue permission' do - expect(policies).not_to include(:update_issue) - end - end + it 'allows guests to read issues' do + expect(permissions(guest, issue)).to include(:read_issue) + expect(permissions(guest, issue)).not_to include(:update_issue, :admin_issue) + + expect(permissions(guest, issue_no_assignee)).to include(:read_issue) + expect(permissions(guest, issue_no_assignee)).not_to include(:update_issue, :admin_issue) + end + + it 'allows reporters to read, update, and admin issues' do + expect(permissions(reporter, issue)).to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter, issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows reporters from group links to read, update, and admin issues' do + expect(permissions(reporter_from_group_link, issue)).to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter_from_group_link, issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) + end - context 'with a user that is a project member' do - let(:policies) { described_class.abilities(user, issue).to_set } + it 'allows issue authors to read and update their issues' do + expect(permissions(author, issue)).to include(:read_issue, :update_issue) + expect(permissions(author, issue)).not_to include(:admin_issue) - before do - issue.project.team << [user, :reporter] - end + expect(permissions(author, issue_no_assignee)).to include(:read_issue) + expect(permissions(author, issue_no_assignee)).not_to include(:update_issue, :admin_issue) + end + + it 'allows issue assignees to read and update their issues' do + expect(permissions(assignee, issue)).to include(:read_issue, :update_issue) + expect(permissions(assignee, issue)).not_to include(:admin_issue) - it 'includes the read_issue permission' do - expect(policies).to include(:read_issue) - end + expect(permissions(assignee, issue_no_assignee)).to include(:read_issue) + expect(permissions(assignee, issue_no_assignee)).not_to include(:update_issue, :admin_issue) + end - it 'includes the admin_issue permission' do - expect(policies).to include(:admin_issue) - end + context 'with confidential issues' do + let(:confidential_issue) { create(:issue, :confidential, project: project, assignee: assignee, author: author) } + let(:confidential_issue_no_assignee) { create(:issue, :confidential, project: project) } - it 'includes the update_issue permission' do - expect(policies).to include(:update_issue) - end + it 'does not allow guests to read confidential issues' do + expect(permissions(guest, confidential_issue)).not_to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(guest, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) end - context 'without a user' do - let(:policies) { described_class.abilities(nil, issue).to_set } + it 'allows reporters to read, update, and admin confidential issues' do + expect(permissions(reporter, confidential_issue)).to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter, confidential_issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) + end + + it 'allows reporter from group links to read, update, and admin confidential issues' do + expect(permissions(reporter_from_group_link, confidential_issue)).to include(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter_from_group_link, confidential_issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) + end - it 'does not include the read_issue permission' do - expect(policies).not_to include(:read_issue) - end + it 'allows issue authors to read and update their confidential issues' do + expect(permissions(author, confidential_issue)).to include(:read_issue, :update_issue) + expect(permissions(author, confidential_issue)).not_to include(:admin_issue) + + expect(permissions(author, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) + end - it 'does not include the admin_issue permission' do - expect(policies).not_to include(:admin_issue) - end + it 'allows issue assignees to read and update their confidential issues' do + expect(permissions(assignee, confidential_issue)).to include(:read_issue, :update_issue) + expect(permissions(assignee, confidential_issue)).not_to include(:admin_issue) - it 'does not include the update_issue permission' do - expect(policies).not_to include(:update_issue) - end + expect(permissions(assignee, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) end end end diff --git a/spec/policies/issues_policy_spec.rb b/spec/policies/issues_policy_spec.rb deleted file mode 100644 index 2b7b6cad654..00000000000 --- a/spec/policies/issues_policy_spec.rb +++ /dev/null @@ -1,193 +0,0 @@ -require 'spec_helper' - -describe IssuePolicy, models: true do - let(:guest) { create(:user) } - let(:author) { create(:user) } - let(:assignee) { create(:user) } - let(:reporter) { create(:user) } - let(:group) { create(:group, :public) } - let(:reporter_from_group_link) { create(:user) } - - def permissions(user, issue) - IssuePolicy.abilities(user, issue).to_set - end - - context 'a private project' do - let(:non_member) { create(:user) } - let(:project) { create(:empty_project, :private) } - let(:issue) { create(:issue, project: project, assignee: assignee, author: author) } - let(:issue_no_assignee) { create(:issue, project: project) } - - before do - project.team << [guest, :guest] - project.team << [author, :guest] - project.team << [assignee, :guest] - project.team << [reporter, :reporter] - - group.add_reporter(reporter_from_group_link) - - create(:project_group_link, group: group, project: project) - end - - it 'does not allow non-members to read issues' do - expect(permissions(non_member, issue)).not_to include(:read_issue, :update_issue, :admin_issue) - expect(permissions(non_member, issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) - end - - it 'allows guests to read issues' do - expect(permissions(guest, issue)).to include(:read_issue) - expect(permissions(guest, issue)).not_to include(:update_issue, :admin_issue) - - expect(permissions(guest, issue_no_assignee)).to include(:read_issue) - expect(permissions(guest, issue_no_assignee)).not_to include(:update_issue, :admin_issue) - end - - it 'allows reporters to read, update, and admin issues' do - expect(permissions(reporter, issue)).to include(:read_issue, :update_issue, :admin_issue) - expect(permissions(reporter, issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) - end - - it 'allows reporters from group links to read, update, and admin issues' do - expect(permissions(reporter_from_group_link, issue)).to include(:read_issue, :update_issue, :admin_issue) - expect(permissions(reporter_from_group_link, issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) - end - - it 'allows issue authors to read and update their issues' do - expect(permissions(author, issue)).to include(:read_issue, :update_issue) - expect(permissions(author, issue)).not_to include(:admin_issue) - - expect(permissions(author, issue_no_assignee)).to include(:read_issue) - expect(permissions(author, issue_no_assignee)).not_to include(:update_issue, :admin_issue) - end - - it 'allows issue assignees to read and update their issues' do - expect(permissions(assignee, issue)).to include(:read_issue, :update_issue) - expect(permissions(assignee, issue)).not_to include(:admin_issue) - - expect(permissions(assignee, issue_no_assignee)).to include(:read_issue) - expect(permissions(assignee, issue_no_assignee)).not_to include(:update_issue, :admin_issue) - end - - context 'with confidential issues' do - let(:confidential_issue) { create(:issue, :confidential, project: project, assignee: assignee, author: author) } - let(:confidential_issue_no_assignee) { create(:issue, :confidential, project: project) } - - it 'does not allow non-members to read confidential issues' do - expect(permissions(non_member, confidential_issue)).not_to include(:read_issue, :update_issue, :admin_issue) - expect(permissions(non_member, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) - end - - it 'does not allow guests to read confidential issues' do - expect(permissions(guest, confidential_issue)).not_to include(:read_issue, :update_issue, :admin_issue) - expect(permissions(guest, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) - end - - it 'allows reporters to read, update, and admin confidential issues' do - expect(permissions(reporter, confidential_issue)).to include(:read_issue, :update_issue, :admin_issue) - expect(permissions(reporter, confidential_issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) - end - - it 'allows reporters from group links to read, update, and admin confidential issues' do - expect(permissions(reporter_from_group_link, confidential_issue)).to include(:read_issue, :update_issue, :admin_issue) - expect(permissions(reporter_from_group_link, confidential_issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) - end - - it 'allows issue authors to read and update their confidential issues' do - expect(permissions(author, confidential_issue)).to include(:read_issue, :update_issue) - expect(permissions(author, confidential_issue)).not_to include(:admin_issue) - - expect(permissions(author, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) - end - - it 'allows issue assignees to read and update their confidential issues' do - expect(permissions(assignee, confidential_issue)).to include(:read_issue, :update_issue) - expect(permissions(assignee, confidential_issue)).not_to include(:admin_issue) - - expect(permissions(assignee, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) - end - end - end - - context 'a public project' do - let(:project) { create(:empty_project, :public) } - let(:issue) { create(:issue, project: project, assignee: assignee, author: author) } - let(:issue_no_assignee) { create(:issue, project: project) } - - before do - project.team << [guest, :guest] - project.team << [reporter, :reporter] - - group.add_reporter(reporter_from_group_link) - - create(:project_group_link, group: group, project: project) - end - - it 'allows guests to read issues' do - expect(permissions(guest, issue)).to include(:read_issue) - expect(permissions(guest, issue)).not_to include(:update_issue, :admin_issue) - - expect(permissions(guest, issue_no_assignee)).to include(:read_issue) - expect(permissions(guest, issue_no_assignee)).not_to include(:update_issue, :admin_issue) - end - - it 'allows reporters to read, update, and admin issues' do - expect(permissions(reporter, issue)).to include(:read_issue, :update_issue, :admin_issue) - expect(permissions(reporter, issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) - end - - it 'allows reporters from group links to read, update, and admin issues' do - expect(permissions(reporter_from_group_link, issue)).to include(:read_issue, :update_issue, :admin_issue) - expect(permissions(reporter_from_group_link, issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) - end - - it 'allows issue authors to read and update their issues' do - expect(permissions(author, issue)).to include(:read_issue, :update_issue) - expect(permissions(author, issue)).not_to include(:admin_issue) - - expect(permissions(author, issue_no_assignee)).to include(:read_issue) - expect(permissions(author, issue_no_assignee)).not_to include(:update_issue, :admin_issue) - end - - it 'allows issue assignees to read and update their issues' do - expect(permissions(assignee, issue)).to include(:read_issue, :update_issue) - expect(permissions(assignee, issue)).not_to include(:admin_issue) - - expect(permissions(assignee, issue_no_assignee)).to include(:read_issue) - expect(permissions(assignee, issue_no_assignee)).not_to include(:update_issue, :admin_issue) - end - - context 'with confidential issues' do - let(:confidential_issue) { create(:issue, :confidential, project: project, assignee: assignee, author: author) } - let(:confidential_issue_no_assignee) { create(:issue, :confidential, project: project) } - - it 'does not allow guests to read confidential issues' do - expect(permissions(guest, confidential_issue)).not_to include(:read_issue, :update_issue, :admin_issue) - expect(permissions(guest, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) - end - - it 'allows reporters to read, update, and admin confidential issues' do - expect(permissions(reporter, confidential_issue)).to include(:read_issue, :update_issue, :admin_issue) - expect(permissions(reporter, confidential_issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) - end - - it 'allows reporter from group links to read, update, and admin confidential issues' do - expect(permissions(reporter_from_group_link, confidential_issue)).to include(:read_issue, :update_issue, :admin_issue) - expect(permissions(reporter_from_group_link, confidential_issue_no_assignee)).to include(:read_issue, :update_issue, :admin_issue) - end - - it 'allows issue authors to read and update their confidential issues' do - expect(permissions(author, confidential_issue)).to include(:read_issue, :update_issue) - expect(permissions(author, confidential_issue)).not_to include(:admin_issue) - - expect(permissions(author, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) - end - - it 'allows issue assignees to read and update their confidential issues' do - expect(permissions(assignee, confidential_issue)).to include(:read_issue, :update_issue) - expect(permissions(assignee, confidential_issue)).not_to include(:admin_issue) - - expect(permissions(assignee, confidential_issue_no_assignee)).not_to include(:read_issue, :update_issue, :admin_issue) - end - end - end -end diff --git a/spec/requests/api/api_internal_helpers_spec.rb b/spec/requests/api/api_internal_helpers_spec.rb deleted file mode 100644 index f5265ea60ff..00000000000 --- a/spec/requests/api/api_internal_helpers_spec.rb +++ /dev/null @@ -1,32 +0,0 @@ -require 'spec_helper' - -describe ::API::Helpers::InternalHelpers do - include ::API::Helpers::InternalHelpers - - describe '.clean_project_path' do - project = 'namespace/project' - namespaced = File.join('namespace2', project) - - { - File.join(Dir.pwd, project) => project, - File.join(Dir.pwd, namespaced) => namespaced, - project => project, - namespaced => namespaced, - project + '.git' => project, - namespaced + '.git' => namespaced, - "/" + project => project, - "/" + namespaced => namespaced, - }.each do |project_path, expected| - context project_path do - # Relative and absolute storage paths, with and without trailing / - ['.', './', Dir.pwd, Dir.pwd + '/'].each do |storage_path| - context "storage path is #{storage_path}" do - subject { clean_project_path(project_path, [{ 'path' => storage_path }]) } - - it { is_expected.to eq(expected) } - end - end - end - end - end -end diff --git a/spec/requests/api/doorkeeper_access_spec.rb b/spec/requests/api/doorkeeper_access_spec.rb index b5897b2e346..868fef65c1c 100644 --- a/spec/requests/api/doorkeeper_access_spec.rb +++ b/spec/requests/api/doorkeeper_access_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe API::API do +describe 'doorkeeper access' do let!(:user) { create(:user) } let!(:application) { Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) } let!(:token) { Doorkeeper::AccessToken.create! application_id: application.id, resource_owner_id: user.id, scopes: "api" } diff --git a/spec/requests/api/helpers/internal_helpers_spec.rb b/spec/requests/api/helpers/internal_helpers_spec.rb new file mode 100644 index 00000000000..f5265ea60ff --- /dev/null +++ b/spec/requests/api/helpers/internal_helpers_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe ::API::Helpers::InternalHelpers do + include ::API::Helpers::InternalHelpers + + describe '.clean_project_path' do + project = 'namespace/project' + namespaced = File.join('namespace2', project) + + { + File.join(Dir.pwd, project) => project, + File.join(Dir.pwd, namespaced) => namespaced, + project => project, + namespaced => namespaced, + project + '.git' => project, + namespaced + '.git' => namespaced, + "/" + project => project, + "/" + namespaced => namespaced, + }.each do |project_path, expected| + context project_path do + # Relative and absolute storage paths, with and without trailing / + ['.', './', Dir.pwd, Dir.pwd + '/'].each do |storage_path| + context "storage path is #{storage_path}" do + subject { clean_project_path(project_path, [{ 'path' => storage_path }]) } + + it { is_expected.to eq(expected) } + end + end + end + end + end +end diff --git a/spec/requests/api/oauth_tokens_spec.rb b/spec/requests/api/oauth_tokens_spec.rb index 819df105960..0d56e1f732e 100644 --- a/spec/requests/api/oauth_tokens_spec.rb +++ b/spec/requests/api/oauth_tokens_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe API::API do +describe 'OAuth tokens' do context 'Resource Owner Password Credentials' do def request_oauth_token(user) post '/oauth/token', username: user.username, password: user.password, grant_type: 'password' diff --git a/spec/routing/environments_spec.rb b/spec/routing/environments_spec.rb index ba124de70bb..624f3c43f0a 100644 --- a/spec/routing/environments_spec.rb +++ b/spec/routing/environments_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Projects::EnvironmentsController, :routing do +describe 'environments routing', :routing do let(:project) { create(:empty_project) } let(:environment) do diff --git a/spec/routing/notifications_routing_spec.rb b/spec/routing/notifications_routing_spec.rb index 24592942a96..54ed87b5520 100644 --- a/spec/routing/notifications_routing_spec.rb +++ b/spec/routing/notifications_routing_spec.rb @@ -1,13 +1,11 @@ require "spec_helper" -describe Profiles::NotificationsController do - describe "routing" do - it "routes to #show" do - expect(get("/profile/notifications")).to route_to("profiles/notifications#show") - end +describe "notifications routing" do + it "routes to #show" do + expect(get("/profile/notifications")).to route_to("profiles/notifications#show") + end - it "routes to #update" do - expect(put("/profile/notifications")).to route_to("profiles/notifications#update") - end + it "routes to #update" do + expect(put("/profile/notifications")).to route_to("profiles/notifications#update") end end diff --git a/spec/serializers/analytics_generic_entity_spec.rb b/spec/serializers/analytics_generic_entity_spec.rb deleted file mode 100644 index 68086216ba9..00000000000 --- a/spec/serializers/analytics_generic_entity_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -require 'spec_helper' - -describe AnalyticsIssueEntity do - let(:user) { create(:user) } - let(:entity_hash) do - { - total_time: "172802.724419", - title: "Eos voluptatem inventore in sed.", - iid: "1", - id: "1", - created_at: "2016-11-12 15:04:02.948604", - author: user, - } - end - - let(:project) { create(:empty_project) } - let(:request) { EntityRequest.new(project: project, entity: :merge_request) } - - let(:entity) do - described_class.new(entity_hash, request: request, project: project) - end - - context 'generic entity' do - subject { entity.as_json } - - it 'contains the entity URL' do - expect(subject).to include(:url) - end - - it 'contains the author' do - expect(subject).to include(:author) - end - - it 'does not contain sensitive information' do - expect(subject).not_to include(/token/) - expect(subject).not_to include(/variables/) - end - end -end diff --git a/spec/serializers/analytics_issue_entity_spec.rb b/spec/serializers/analytics_issue_entity_spec.rb new file mode 100644 index 00000000000..68086216ba9 --- /dev/null +++ b/spec/serializers/analytics_issue_entity_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe AnalyticsIssueEntity do + let(:user) { create(:user) } + let(:entity_hash) do + { + total_time: "172802.724419", + title: "Eos voluptatem inventore in sed.", + iid: "1", + id: "1", + created_at: "2016-11-12 15:04:02.948604", + author: user, + } + end + + let(:project) { create(:empty_project) } + let(:request) { EntityRequest.new(project: project, entity: :merge_request) } + + let(:entity) do + described_class.new(entity_hash, request: request, project: project) + end + + context 'generic entity' do + subject { entity.as_json } + + it 'contains the entity URL' do + expect(subject).to include(:url) + end + + it 'contains the author' do + expect(subject).to include(:author) + end + + it 'does not contain sensitive information' do + expect(subject).not_to include(/token/) + expect(subject).not_to include(/variables/) + end + end +end diff --git a/spec/services/issues/resolve_discussions_spec.rb b/spec/services/issues/resolve_discussions_spec.rb index 4a4929daefc..c3b4c2176ee 100644 --- a/spec/services/issues/resolve_discussions_spec.rb +++ b/spec/services/issues/resolve_discussions_spec.rb @@ -1,15 +1,15 @@ require 'spec_helper.rb' -class DummyService < Issues::BaseService - include ::Issues::ResolveDiscussions +describe Issues::ResolveDiscussions, services: true do + class DummyService < Issues::BaseService + include ::Issues::ResolveDiscussions - def initialize(*args) - super - filter_resolve_discussion_params + def initialize(*args) + super + filter_resolve_discussion_params + end end -end -describe DummyService, services: true do let(:project) { create(:project, :repository) } let(:user) { create(:user) } @@ -23,7 +23,7 @@ describe DummyService, services: true do let(:other_merge_request) { create(:merge_request, source_project: project, source_branch: "other") } describe "#merge_request_for_resolving_discussion" do - let(:service) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid) } + let(:service) { DummyService.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid) } it "finds the merge request" do expect(service.merge_request_to_resolve_discussions_of).to eq(merge_request) @@ -43,7 +43,7 @@ describe DummyService, services: true do describe "#discussions_to_resolve" do it "contains a single discussion when matching merge request and discussion are passed" do - service = described_class.new( + service = DummyService.new( project, user, discussion_to_resolve: discussion.id, @@ -61,7 +61,7 @@ describe DummyService, services: true do noteable: merge_request, project: merge_request.target_project, line_number: 15)]) - service = described_class.new( + service = DummyService.new( project, user, merge_request_to_resolve_discussions_of: merge_request.iid @@ -79,7 +79,7 @@ describe DummyService, services: true do project: merge_request.target_project, line_number: 15, )]) - service = described_class.new( + service = DummyService.new( project, user, merge_request_to_resolve_discussions_of: merge_request.iid @@ -92,7 +92,7 @@ describe DummyService, services: true do end it "is empty when a discussion and another merge request are passed" do - service = described_class.new( + service = DummyService.new( project, user, discussion_to_resolve: discussion.id, diff --git a/spec/services/merge_requests/resolved_discussion_notification_service.rb b/spec/services/merge_requests/resolved_discussion_notification_service.rb deleted file mode 100644 index 7ddd812e513..00000000000 --- a/spec/services/merge_requests/resolved_discussion_notification_service.rb +++ /dev/null @@ -1,46 +0,0 @@ -require 'spec_helper' - -describe MergeRequests::ResolvedDiscussionNotificationService, services: true do - let(:merge_request) { create(:merge_request) } - let(:user) { create(:user) } - let(:project) { merge_request.project } - subject { described_class.new(project, user) } - - describe "#execute" do - context "when not all discussions are resolved" do - before do - allow(merge_request).to receive(:discussions_resolved?).and_return(false) - end - - it "doesn't add a system note" do - expect(SystemNoteService).not_to receive(:resolve_all_discussions) - - subject.execute(merge_request) - end - - it "doesn't send a notification email" do - expect_any_instance_of(NotificationService).not_to receive(:resolve_all_discussions) - - subject.execute(merge_request) - end - end - - context "when all discussions are resolved" do - before do - allow(merge_request).to receive(:discussions_resolved?).and_return(true) - end - - it "adds a system note" do - expect(SystemNoteService).to receive(:resolve_all_discussions).with(merge_request, project, user) - - subject.execute(merge_request) - end - - it "sends a notification email" do - expect_any_instance_of(NotificationService).to receive(:resolve_all_discussions).with(merge_request, user) - - subject.execute(merge_request) - end - end - end -end diff --git a/spec/services/merge_requests/resolved_discussion_notification_service_spec.rb b/spec/services/merge_requests/resolved_discussion_notification_service_spec.rb new file mode 100644 index 00000000000..7ddd812e513 --- /dev/null +++ b/spec/services/merge_requests/resolved_discussion_notification_service_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe MergeRequests::ResolvedDiscussionNotificationService, services: true do + let(:merge_request) { create(:merge_request) } + let(:user) { create(:user) } + let(:project) { merge_request.project } + subject { described_class.new(project, user) } + + describe "#execute" do + context "when not all discussions are resolved" do + before do + allow(merge_request).to receive(:discussions_resolved?).and_return(false) + end + + it "doesn't add a system note" do + expect(SystemNoteService).not_to receive(:resolve_all_discussions) + + subject.execute(merge_request) + end + + it "doesn't send a notification email" do + expect_any_instance_of(NotificationService).not_to receive(:resolve_all_discussions) + + subject.execute(merge_request) + end + end + + context "when all discussions are resolved" do + before do + allow(merge_request).to receive(:discussions_resolved?).and_return(true) + end + + it "adds a system note" do + expect(SystemNoteService).to receive(:resolve_all_discussions).with(merge_request, project, user) + + subject.execute(merge_request) + end + + it "sends a notification email" do + expect_any_instance_of(NotificationService).to receive(:resolve_all_discussions).with(merge_request, user) + + subject.execute(merge_request) + end + end + end +end diff --git a/spec/workers/pipeline_proccess_worker_spec.rb b/spec/workers/pipeline_proccess_worker_spec.rb deleted file mode 100644 index 86e9d7f6684..00000000000 --- a/spec/workers/pipeline_proccess_worker_spec.rb +++ /dev/null @@ -1,22 +0,0 @@ -require 'spec_helper' - -describe PipelineProcessWorker do - describe '#perform' do - context 'when pipeline exists' do - let(:pipeline) { create(:ci_pipeline) } - - it 'processes pipeline' do - expect_any_instance_of(Ci::Pipeline).to receive(:process!) - - described_class.new.perform(pipeline.id) - end - end - - context 'when pipeline does not exist' do - it 'does not raise exception' do - expect { described_class.new.perform(123) } - .not_to raise_error - end - end - end -end diff --git a/spec/workers/pipeline_process_worker_spec.rb b/spec/workers/pipeline_process_worker_spec.rb new file mode 100644 index 00000000000..86e9d7f6684 --- /dev/null +++ b/spec/workers/pipeline_process_worker_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe PipelineProcessWorker do + describe '#perform' do + context 'when pipeline exists' do + let(:pipeline) { create(:ci_pipeline) } + + it 'processes pipeline' do + expect_any_instance_of(Ci::Pipeline).to receive(:process!) + + described_class.new.perform(pipeline.id) + end + end + + context 'when pipeline does not exist' do + it 'does not raise exception' do + expect { described_class.new.perform(123) } + .not_to raise_error + end + end + end +end -- cgit v1.2.1