diff options
author | Simon Knox <psimyn@gmail.com> | 2017-11-07 18:29:14 +1100 |
---|---|---|
committer | Simon Knox <psimyn@gmail.com> | 2017-11-07 18:29:14 +1100 |
commit | 8b78c1e5726f21147ba05377f241f03b3810d41f (patch) | |
tree | 6d38e8e427e7de58f51d55e8c64804050fef1b63 /spec | |
parent | e9fb244ad3fdbb42585c1455302eec91af5e11d2 (diff) | |
parent | e99ddb6f374c9f79c1c78e808c5e9bd983bed227 (diff) | |
download | gitlab-ce-8b78c1e5726f21147ba05377f241f03b3810d41f.tar.gz |
Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce into 38394-smarter-interval
Diffstat (limited to 'spec')
105 files changed, 3066 insertions, 1548 deletions
diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb index 4aed2a25baa..9e8a37171ec 100644 --- a/spec/controllers/metrics_controller_spec.rb +++ b/spec/controllers/metrics_controller_spec.rb @@ -67,7 +67,8 @@ describe MetricsController do it 'returns proper response' do get :index - expect(response.status).to eq(404) + expect(response.status).to eq(200) + expect(response.body).to eq("# Metrics are disabled, see: http://test.host/help/administration/monitoring/prometheus/gitlab_metrics#gitlab-prometheus-metrics\n") end end end diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index bd924a1c7be..de4cf40b492 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -1,68 +1,108 @@ require 'spec_helper' describe Projects::ClustersController do - set(:user) { create(:user) } - set(:project) { create(:project) } - let(:role) { :master } + include AccessMatchersForController + include GoogleApi::CloudPlatformHelpers - before do - project.team << [user, role] + describe 'GET index' do + describe 'functionality' do + let(:user) { create(:user) } - sign_in(user) - end + before do + project.add_master(user) + sign_in(user) + end - describe 'GET index' do - subject do - get :index, namespace_id: project.namespace, - project_id: project - end + context 'when project has a cluster' do + let(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:project) { cluster.project } - context 'when cluster is already created' do - let!(:cluster) { create(:gcp_cluster, :created_on_gke, project: project) } + it { expect(go).to redirect_to(project_cluster_path(project, project.cluster)) } + end - it 'redirects to show a cluster' do - subject + context 'when project does not have a cluster' do + let(:project) { create(:project) } - expect(response).to redirect_to(project_cluster_path(project, cluster)) + it { expect(go).to redirect_to(new_project_cluster_path(project)) } end end - context 'when we do not have cluster' do - it 'redirects to create a cluster' do - subject + describe 'security' do + let(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:project) { cluster.project } + + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end - expect(response).to redirect_to(new_project_cluster_path(project)) - end + def go + get :index, namespace_id: project.namespace.to_param, project_id: project end end describe 'GET login' do - render_views + let(:project) { create(:project) } - subject do - get :login, namespace_id: project.namespace, - project_id: project - end - - context 'when we do have omniauth configured' do - it 'shows login button' do - subject + describe 'functionality' do + let(:user) { create(:user) } - expect(response.body).to include('auth_buttons/signin_with_google') + before do + project.add_master(user) + sign_in(user) end - end - context 'when we do not have omniauth configured' do - before do - stub_omniauth_setting(providers: []) + context 'when omniauth has been configured' do + let(:key) { 'secere-key' } + + let(:session_key_for_redirect_uri) do + GoogleApi::CloudPlatform::Client.session_key_for_redirect_uri(key) + end + + before do + allow(SecureRandom).to receive(:hex).and_return(key) + end + + it 'has authorize_url' do + go + + expect(assigns(:authorize_url)).to include(key) + expect(session[session_key_for_redirect_uri]).to eq(project_clusters_url(project)) + end end - it 'shows notice message' do - subject + context 'when omniauth has not configured' do + before do + stub_omniauth_setting(providers: []) + end + + it 'does not have authorize_url' do + go - expect(response.body).to include('Ask your GitLab administrator if you want to use this service.') + expect(assigns(:authorize_url)).to be_nil + end end end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + + def go + get :login, namespace_id: project.namespace, project_id: project + end end shared_examples 'requires to login' do @@ -74,235 +114,335 @@ describe Projects::ClustersController do end describe 'GET new' do - render_views + let(:project) { create(:project) } - subject do - get :new, namespace_id: project.namespace, - project_id: project - end + describe 'functionality' do + let(:user) { create(:user) } - context 'when logged' do before do - make_logged_in + project.add_master(user) + sign_in(user) + end + + context 'when access token is valid' do + before do + stub_google_api_validate_token + end + + it 'has new object' do + go + + expect(assigns(:cluster)).to be_an_instance_of(Clusters::Cluster) + end end - it 'shows a creation form' do - subject + context 'when access token is expired' do + before do + stub_google_api_expired_token + end + + it { expect(go).to redirect_to(login_project_clusters_path(project)) } + end - expect(response.body).to include('Create cluster') + context 'when access token is not stored in session' do + it { expect(go).to redirect_to(login_project_clusters_path(project)) } end end - context 'when not logged' do - it_behaves_like 'requires to login' + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + + def go + get :new, namespace_id: project.namespace, project_id: project end end describe 'POST create' do - subject do - post :create, params.merge(namespace_id: project.namespace, - project_id: project) + let(:project) { create(:project) } + + let(:params) do + { + cluster: { + name: 'new-cluster', + provider_type: :gcp, + provider_gcp_attributes: { + gcp_project_id: '111' + } + } + } end - context 'when not logged' do - let(:params) { {} } - - it_behaves_like 'requires to login' - end + describe 'functionality' do + let(:user) { create(:user) } - context 'when logged in' do before do - make_logged_in + project.add_master(user) + sign_in(user) end - context 'when all required parameters are set' do - let(:params) do - { - cluster: { - gcp_cluster_name: 'new-cluster', - gcp_project_id: '111' - } - } - end - + context 'when access token is valid' do before do - expect(ClusterProvisionWorker).to receive(:perform_async) { } + stub_google_api_validate_token end - it 'creates a new cluster' do - expect { subject }.to change { Gcp::Cluster.count } - - expect(response).to redirect_to(project_cluster_path(project, project.cluster)) + context 'when creates a cluster on gke' do + it 'creates a new cluster' do + expect(ClusterProvisionWorker).to receive(:perform_async) + expect { go }.to change { Clusters::Cluster.count } + expect(response).to redirect_to(project_cluster_path(project, project.cluster)) + end end end - context 'when not all required parameters are set' do - render_views - - let(:params) do - { - cluster: { - project_namespace: 'some namespace' - } - } + context 'when access token is expired' do + before do + stub_google_api_expired_token end - it 'shows an error message' do - expect { subject }.not_to change { Gcp::Cluster.count } + it 'redirects to login page' do + expect(go).to redirect_to(login_project_clusters_path(project)) + end + end - expect(response).to render_template(:new) + context 'when access token is not stored in session' do + it 'redirects to login page' do + expect(go).to redirect_to(login_project_clusters_path(project)) end end end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + + def go + post :create, params.merge(namespace_id: project.namespace, project_id: project) + end end describe 'GET status' do - let(:cluster) { create(:gcp_cluster, :created_on_gke, project: project) } + let(:cluster) { create(:cluster, :project, :providing_by_gcp) } + let(:project) { cluster.project } + + describe 'functionality' do + let(:user) { create(:user) } - subject do + before do + project.add_master(user) + sign_in(user) + end + + it "responds with matching schema" do + go + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('cluster_status') + end + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + + def go get :status, namespace_id: project.namespace, project_id: project, id: cluster, format: :json end - - it "responds with matching schema" do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('cluster_status') - end end describe 'GET show' do - render_views + let(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:project) { cluster.project } - let(:cluster) { create(:gcp_cluster, :created_on_gke, project: project) } + describe 'functionality' do + let(:user) { create(:user) } - subject do - get :show, namespace_id: project.namespace, - project_id: project, - id: cluster - end - - context 'when logged as master' do - it "allows to update cluster" do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response.body).to include("Save") + before do + project.add_master(user) + sign_in(user) end - it "allows remove integration" do - subject + it "renders view" do + go expect(response).to have_gitlab_http_status(:ok) - expect(response.body).to include("Remove integration") + expect(assigns(:cluster)).to eq(cluster) end end - context 'when logged as developer' do - let(:role) { :developer } - - it "does not allow to access page" do - subject + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end - expect(response).to have_gitlab_http_status(:not_found) - end + def go + get :show, namespace_id: project.namespace, + project_id: project, + id: cluster end end describe 'PUT update' do - render_views + let(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:project) { cluster.project } - let(:service) { project.build_kubernetes_service } - let(:cluster) { create(:gcp_cluster, :created_on_gke, project: project, service: service) } - let(:params) { {} } + describe 'functionality' do + let(:user) { create(:user) } - subject do - put :update, params.merge(namespace_id: project.namespace, - project_id: project, - id: cluster) - end + before do + project.add_master(user) + sign_in(user) + end - context 'when logged as master' do - context 'when valid params are used' do + context 'when update enabled' do let(:params) do { cluster: { enabled: false } } end - it "redirects back to show page" do - subject + it "updates and redirects back to show page" do + go + cluster.reload expect(response).to redirect_to(project_cluster_path(project, project.cluster)) expect(flash[:notice]).to eq('Cluster was successfully updated.') + expect(cluster.enabled).to be_falsey end - end - context 'when invalid params are used' do - let(:params) do - { - cluster: { project_namespace: 'my Namespace 321321321 #' } - } - end + context 'when cluster is being created' do + let(:cluster) { create(:cluster, :project, :providing_by_gcp) } - it "rejects changes" do - subject + it "rejects changes" do + go - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:show) + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:show) + expect(cluster.enabled).to be_truthy + end end end end - context 'when logged as developer' do - let(:role) { :developer } + describe 'security' do + let(:params) do + { + cluster: { enabled: false } + } + end - it "does not allow to update cluster" do - subject + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end - expect(response).to have_gitlab_http_status(:not_found) - end + def go + put :update, params.merge(namespace_id: project.namespace, + project_id: project, + id: cluster) end end describe 'delete update' do - let(:cluster) { create(:gcp_cluster, :created_on_gke, project: project) } + let(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:project) { cluster.project } - subject do - delete :destroy, namespace_id: project.namespace, - project_id: project, - id: cluster - end + describe 'functionality' do + let(:user) { create(:user) } - context 'when logged as master' do - it "redirects back to clusters list" do - subject + before do + project.add_master(user) + sign_in(user) + end + + it "destroys and redirects back to clusters list" do + expect { go } + .to change { Clusters::Cluster.count }.by(-1) + .and change { Clusters::Platforms::Kubernetes.count }.by(-1) + .and change { Clusters::Providers::Gcp.count }.by(-1) expect(response).to redirect_to(project_clusters_path(project)) expect(flash[:notice]).to eq('Cluster integration was successfully removed.') end - end - context 'when logged as developer' do - let(:role) { :developer } + context 'when cluster is being created' do + let(:cluster) { create(:cluster, :project, :providing_by_gcp) } + + it "destroys and redirects back to clusters list" do + expect { go } + .to change { Clusters::Cluster.count }.by(-1) + .and change { Clusters::Providers::Gcp.count }.by(-1) + + expect(response).to redirect_to(project_clusters_path(project)) + expect(flash[:notice]).to eq('Cluster integration was successfully removed.') + end + end + + context 'when provider is user' do + let(:cluster) { create(:cluster, :project, :provided_by_user) } - it "does not allow to destroy cluster" do - subject + it "destroys and redirects back to clusters list" do + expect { go } + .to change { Clusters::Cluster.count }.by(-1) + .and change { Clusters::Platforms::Kubernetes.count }.by(-1) + .and change { Clusters::Providers::Gcp.count }.by(0) - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to redirect_to(project_clusters_path(project)) + expect(flash[:notice]).to eq('Cluster integration was successfully removed.') + end end end - end - def make_logged_in - session[GoogleApi::CloudPlatform::Client.session_key_for_token] = '1234' - session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] = in_hour.to_i.to_s - end + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end - def in_hour - Time.now + 1.hour + def go + delete :destroy, namespace_id: project.namespace, + project_id: project, + id: cluster + end end end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 8016176110e..4dbbaecdd6d 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -861,7 +861,7 @@ describe Projects::IssuesController do end it 'delegates the update of the todos count cache to TodoService' do - expect_any_instance_of(TodoService).to receive(:destroy_issue).with(issue, owner).once + expect_any_instance_of(TodoService).to receive(:destroy_issuable).with(issue, owner).once delete :destroy, namespace_id: project.namespace, project_id: project, id: issue.iid end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 14021b8ca50..bfdad85c082 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -456,7 +456,7 @@ describe Projects::MergeRequestsController do end it 'delegates the update of the todos count cache to TodoService' do - expect_any_instance_of(TodoService).to receive(:destroy_merge_request).with(merge_request, owner).once + expect_any_instance_of(TodoService).to receive(:destroy_issuable).with(merge_request, owner).once delete :destroy, namespace_id: project.namespace, project_id: project, id: merge_request.iid end diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 43afcc14957..5f5a789d5cc 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -59,6 +59,7 @@ describe Projects::NotesController do expect(note_json[:id]).to eq(note.id) expect(note_json[:discussion_html]).not_to be_nil expect(note_json[:diff_discussion_html]).to be_nil + expect(note_json[:discussion_line_code]).to be_nil end end @@ -74,6 +75,7 @@ describe Projects::NotesController do expect(note_json[:id]).to eq(note.id) expect(note_json[:discussion_html]).not_to be_nil expect(note_json[:diff_discussion_html]).not_to be_nil + expect(note_json[:discussion_line_code]).not_to be_nil end end @@ -92,6 +94,7 @@ describe Projects::NotesController do expect(note_json[:id]).to eq(note.id) expect(note_json[:discussion_html]).not_to be_nil expect(note_json[:diff_discussion_html]).to be_nil + expect(note_json[:discussion_line_code]).to be_nil end end @@ -104,6 +107,7 @@ describe Projects::NotesController do expect(note_json[:id]).to eq(note.id) expect(note_json[:discussion_html]).to be_nil expect(note_json[:diff_discussion_html]).to be_nil + expect(note_json[:discussion_line_code]).to be_nil end context 'when user cannot read commit' do @@ -133,6 +137,7 @@ describe Projects::NotesController do expect(note_json[:html]).not_to be_nil expect(note_json[:discussion_html]).to be_nil expect(note_json[:diff_discussion_html]).to be_nil + expect(note_json[:discussion_line_code]).to be_nil end end diff --git a/spec/factories/clusters/cluster.rb b/spec/factories/clusters/cluster.rb new file mode 100644 index 00000000000..c4261178f2d --- /dev/null +++ b/spec/factories/clusters/cluster.rb @@ -0,0 +1,39 @@ +FactoryGirl.define do + factory :cluster, class: Clusters::Cluster do + user + name 'test-cluster' + + trait :project do + after(:create) do |cluster, evaluator| + cluster.projects << create(:project) + end + end + + trait :provided_by_user do + provider_type :user + platform_type :kubernetes + + platform_kubernetes do + create(:cluster_platform_kubernetes, :configured) + end + end + + trait :provided_by_gcp do + provider_type :gcp + platform_type :kubernetes + + before(:create) do |cluster, evaluator| + cluster.platform_kubernetes = build(:cluster_platform_kubernetes, :configured) + cluster.provider_gcp = build(:cluster_provider_gcp, :created) + end + end + + trait :providing_by_gcp do + provider_type :gcp + + provider_gcp do + create(:cluster_provider_gcp, :creating) + end + end + end +end diff --git a/spec/factories/clusters/platforms/kubernetes.rb b/spec/factories/clusters/platforms/kubernetes.rb new file mode 100644 index 00000000000..8b3e6ff35fa --- /dev/null +++ b/spec/factories/clusters/platforms/kubernetes.rb @@ -0,0 +1,20 @@ +FactoryGirl.define do + factory :cluster_platform_kubernetes, class: Clusters::Platforms::Kubernetes do + cluster + namespace nil + api_url 'https://kubernetes.example.com' + token 'a' * 40 + + trait :configured do + api_url 'https://kubernetes.example.com' + token 'a' * 40 + username 'xxxxxx' + password 'xxxxxx' + + after(:create) do |platform_kubernetes, evaluator| + pem_file = File.expand_path(Rails.root.join('spec/fixtures/clusters/sample_cert.pem')) + platform_kubernetes.ca_cert = File.read(pem_file) + end + end + end +end diff --git a/spec/factories/clusters/providers/gcp.rb b/spec/factories/clusters/providers/gcp.rb new file mode 100644 index 00000000000..a815410512a --- /dev/null +++ b/spec/factories/clusters/providers/gcp.rb @@ -0,0 +1,32 @@ +FactoryGirl.define do + factory :cluster_provider_gcp, class: Clusters::Providers::Gcp do + cluster + gcp_project_id 'test-gcp-project' + + trait :scheduled do + access_token 'access_token_123' + end + + trait :creating do + access_token 'access_token_123' + + after(:build) do |gcp, evaluator| + gcp.make_creating('operation-123') + end + end + + trait :created do + endpoint '111.111.111.111' + + after(:build) do |gcp, evaluator| + gcp.make_created + end + end + + trait :errored do + after(:build) do |gcp, evaluator| + gcp.make_errored('Something wrong') + end + end + end +end diff --git a/spec/factories/gcp/cluster.rb b/spec/factories/gcp/cluster.rb deleted file mode 100644 index 630e40da888..00000000000 --- a/spec/factories/gcp/cluster.rb +++ /dev/null @@ -1,38 +0,0 @@ -FactoryGirl.define do - factory :gcp_cluster, class: Gcp::Cluster do - project - user - enabled true - gcp_project_id 'gcp-project-12345' - gcp_cluster_name 'test-cluster' - gcp_cluster_zone 'us-central1-a' - gcp_cluster_size 1 - gcp_machine_type 'n1-standard-4' - - trait :with_kubernetes_service do - after(:create) do |cluster, evaluator| - create(:kubernetes_service, project: cluster.project).tap do |service| - cluster.update(service: service) - end - end - end - - trait :custom_project_namespace do - project_namespace 'sample-app' - end - - trait :created_on_gke do - status_event :make_created - endpoint '111.111.111.111' - ca_cert 'xxxxxx' - kubernetes_token 'xxxxxx' - username 'xxxxxx' - password 'xxxxxx' - end - - trait :errored do - status_event :make_errored - status_reason 'general error' - end - end -end diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 7c4a22c94c2..cc6cef63b47 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -83,10 +83,10 @@ FactoryGirl.define do target_project = merge_request.target_project source_project = merge_request.source_project - # Fake `write_ref` if we don't have repository + # Fake `fetch_ref!` if we don't have repository # We have too many existing tests replying on this behaviour unless [target_project, source_project].all?(&:repository_exists?) - allow(merge_request).to receive(:write_ref) + allow(merge_request).to receive(:fetch_ref!) end end diff --git a/spec/features/copy_as_gfm_spec.rb b/spec/features/copy_as_gfm_spec.rb index c6ba1211b9e..1fcb8d5bc67 100644 --- a/spec/features/copy_as_gfm_spec.rb +++ b/spec/features/copy_as_gfm_spec.rb @@ -664,7 +664,7 @@ describe 'Copy as GFM', :js do def html_to_gfm(html, transformer = 'transformGFMSelection', target: nil) js = <<-JS.strip_heredoc (function(html) { - var transformer = window.gl.CopyAsGFM[#{transformer.inspect}]; + var transformer = window.CopyAsGFM[#{transformer.inspect}]; var node = document.createElement('div'); $(html).each(function() { node.appendChild(this) }); @@ -678,7 +678,7 @@ describe 'Copy as GFM', :js do node = transformer(node, target); if (!node) return null; - return window.gl.CopyAsGFM.nodeToGFM(node); + return window.CopyAsGFM.nodeToGFM(node); })("#{escape_javascript(html)}") JS page.evaluate_script(js) diff --git a/spec/features/issues/create_branch_merge_request_spec.rb b/spec/features/issues/create_branch_merge_request_spec.rb index 546dc7e8a49..edea95c6699 100644 --- a/spec/features/issues/create_branch_merge_request_spec.rb +++ b/spec/features/issues/create_branch_merge_request_spec.rb @@ -64,6 +64,19 @@ feature 'Create Branch/Merge Request Dropdown on issue page', :feature, :js do end end + context 'when merge requests are disabled' do + before do + project.project_feature.update(merge_requests_access_level: 0) + + visit project_issue_path(project, issue) + end + + it 'shows only create branch button' do + expect(page).not_to have_button('Create a merge request') + expect(page).to have_button('Create a branch') + end + end + context 'when issue is confidential' do it 'disables the create branch button' do issue = create(:issue, :confidential, project: project) diff --git a/spec/features/projects/clusters_spec.rb b/spec/features/projects/clusters_spec.rb index 810f2c39b43..27c1f5062f5 100644 --- a/spec/features/projects/clusters_spec.rb +++ b/spec/features/projects/clusters_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' feature 'Clusters', :js do + include GoogleApi::CloudPlatformHelpers + let!(:project) { create(:project, :repository) } let!(:user) { create(:user) } @@ -11,8 +13,10 @@ feature 'Clusters', :js do context 'when user has signed in Google' do before do - allow_any_instance_of(GoogleApi::CloudPlatform::Client) - .to receive(:validate_token).and_return(true) + allow_any_instance_of(Projects::ClustersController) + .to receive(:token_in_session).and_return('token') + allow_any_instance_of(Projects::ClustersController) + .to receive(:expires_at_in_session).and_return(1.hour.since.to_i.to_s) end context 'when user does not have a cluster and visits cluster index page' do @@ -36,15 +40,15 @@ feature 'Clusters', :js do allow(WaitForClusterCreationWorker).to receive(:perform_in).and_return(nil) - fill_in 'cluster_gcp_project_id', with: 'gcp-project-123' - fill_in 'cluster_gcp_cluster_name', with: 'dev-cluster' + fill_in 'cluster_provider_gcp_attributes_gcp_project_id', with: 'gcp-project-123' + fill_in 'cluster_name', with: 'dev-cluster' click_button 'Create cluster' end it 'user sees a cluster details page and creation status' do expect(page).to have_content('Cluster is being created on Google Container Engine...') - Gcp::Cluster.last.make_created! + Clusters::Cluster.last.provider.make_created! expect(page).to have_content('Cluster was successfully created on Google Container Engine') end @@ -62,7 +66,8 @@ feature 'Clusters', :js do end context 'when user has a cluster and visits cluster index page' do - let!(:cluster) { create(:gcp_cluster, :created_on_gke, :with_kubernetes_service, project: project) } + let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:project) { cluster.project } before do visit project_clusters_path(project) @@ -70,7 +75,7 @@ feature 'Clusters', :js do it 'user sees an cluster details page' do expect(page).to have_button('Save') - expect(page.find(:css, '.cluster-name').value).to eq(cluster.gcp_cluster_name) + expect(page.find(:css, '.cluster-name').value).to eq(cluster.name) end context 'when user disables the cluster' do diff --git a/spec/fixtures/clusters/sample_cert.pem b/spec/fixtures/clusters/sample_cert.pem new file mode 100644 index 00000000000..e39a2b34416 --- /dev/null +++ b/spec/fixtures/clusters/sample_cert.pem @@ -0,0 +1,33 @@ +-----BEGIN CERTIFICATE----- +MIIFtTCCA52gAwIBAgIJAOutg3Kf2y5dMA0GCSqGSIb3DQEBBQUAMEUxCzAJBgNV +BAYTAkFVMRMwEQYDVQQIEwpTb21lLVN0YXRlMSEwHwYDVQQKExhJbnRlcm5ldCBX +aWRnaXRzIFB0eSBMdGQwHhcNMTcxMDI5MTgxOTU3WhcNMTgxMDI5MTgxOTU3WjBF +MQswCQYDVQQGEwJBVTETMBEGA1UECBMKU29tZS1TdGF0ZTEhMB8GA1UEChMYSW50 +ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIIC +CgKCAgEAvQysroM3TLxaavadSPnFIltrYnxCnU4PvCR8971HMWXsq7Z4ShU4BbbE +8yp7oUFjulSwW6DhdIvnQb8ihLKictLmrA0isQqrD/iNpKZ6/lI4DGWw4QzrvMnW +V4yy2QZNpg9tzQHd4+xkeeIoG23RijDU/sPd5dqxF+rPHBfCVInmYvSzLvMhneNj +Bt6gV02gU9e9hsnMatsDvEbvWKp7wcbPot0nWrfZulx2QAWyXy+zG9mJQUds6yc0 +4agAeT9JEb/xtRgR/kS0aUHSGnfSnhZiEn17s0PhTmbu7qSHgzgB+7oJrC9jPoUh +S2Wo3n0xykAjHrA8wC/Ddw3L38S41VQ58GEfNchistPswyMmXo/Oenv9P3s/kCOI +fndiksFNdqVo51y9Vjngj589hpOseFDyKmWPIEQZ9kxW/crjP6RZWWLHgz26KtxZ +uJaoYL8VBbYfrk/bucw0Ma2GEOp8rTsBE7SvgejXZa78q+381Kzc/utW6VwSXqzY +xeIitft0rXi17SZ+XoiTkIXtHn0ZwMtOXNDBADTpFmKa6wVACQilvcpOYD8gUHyH +pB+EDRdST3M4Fiq1MBAVhk8Lj3tHSJ/1ymeF1PWSu57AnJlzerzq2fcfPotNNd37 +ZPNkPh0kxPLwxbAyrHflzx9qVVdI1irY9055mNSnhzlec4qJ9cECAwEAAaOBpzCB +pDAdBgNVHQ4EFgQUnVa5dYPoIG/3+qXml0bX8+N16GwwdQYDVR0jBG4wbIAUnVa5 +dYPoIG/3+qXml0bX8+N16GyhSaRHMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIEwpT +b21lLVN0YXRlMSEwHwYDVQQKExhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGSCCQDr +rYNyn9suXTAMBgNVHRMEBTADAQH/MA0GCSqGSIb3DQEBBQUAA4ICAQAUg4cyxXi1 +VR8ejTpaAruRyJ1pEG9Kc3kiIRXODy60z3hJXnx9LkScPkWGiuL5XacfZ2rMd4bw +oVXIyi8U1UHWfAH8EZdrFKkU92jCiL5soHUONxLAvQEJ/FTR/qijrpzLCxXBdVQE +xFEDWUu6rxLFyjEwzwnRTLgpjR606fdb7qXHkuAMvZ/ezJj8j97hok3Odpn4lr2H +6hMTpK7HmDBX+kmdJJ+yBrm9hG1Pzpl7QU0dkxZ+qJNFjYMLnziiTwkv0c5ZaA9E +NykZUcOv3Sjb6spu1A/E2BSq4WTjkIjrogFlfimE1vmUmObTRJOqUB0Vky1kHEwN +pg7QqIJQmof1EAIaSM/YpUWXyumBwGLDUEud1JUz05In9Q4IZjEwZSJwbQW4fUia +A93m9rk3Lw3xsFcaUdPMFIXk0rPoF1IgmV/oqb0gK95lOWRLbN+AV8qpKPpcKXOc +TkIdFE47ZisEDhIdF6wC1izEMLeMEsPAO7/Y6MY4nRxsinSe95lRaw+yQpzx+mvJ +Q7n1kiHI9Pd5M3+CiQda0d/GO1o5ORJnUGJRvr9HKuNmE7Lif0As/N0AlywjzE7A +6Z8AEiWyRV1ffshu1k2UKmzvZuZeGGKRtrIjbJIRAtpRVtVZZGzhq5/sojCLoJ+u +texqFBUo/4mFRZa4pDItUdyOlDy2/LO/ag== +-----END CERTIFICATE----- diff --git a/spec/helpers/events_helper_spec.rb b/spec/helpers/events_helper_spec.rb index d5536fcb22b..8a80b88da5d 100644 --- a/spec/helpers/events_helper_spec.rb +++ b/spec/helpers/events_helper_spec.rb @@ -1,96 +1,6 @@ require 'spec_helper' describe EventsHelper do - describe '#event_note' do - let(:user) { build(:user) } - - before do - allow(helper).to receive(:current_user).and_return(user) - end - - it 'displays one line of plain text without alteration' do - input = 'A short, plain note' - expect(helper.event_note(input)).to match(input) - expect(helper.event_note(input)).not_to match(/\.\.\.\z/) - end - - it 'displays inline code' do - input = 'A note with `inline code`' - expected = 'A note with <code>inline code</code>' - - expect(helper.event_note(input)).to match(expected) - end - - it 'truncates a note with multiple paragraphs' do - input = "Paragraph 1\n\nParagraph 2" - expected = 'Paragraph 1...' - - expect(helper.event_note(input)).to match(expected) - end - - it 'displays the first line of a code block' do - input = "```\nCode block\nwith two lines\n```" - expected = %r{<pre.+><code><span class="line">Code block\.\.\.</span>\n</code></pre>} - - expect(helper.event_note(input)).to match(expected) - end - - it 'truncates a single long line of text' do - text = 'The quick brown fox jumped over the lazy dog twice' # 50 chars - input = text * 4 - expected = (text * 2).sub(/.{3}/, '...') - - expect(helper.event_note(input)).to match(expected) - end - - it 'preserves a link href when link text is truncated' do - text = 'The quick brown fox jumped over the lazy dog' # 44 chars - input = "#{text}#{text}#{text} " # 133 chars - link_url = 'http://example.com/foo/bar/baz' # 30 chars - input << link_url - expected_link_text = 'http://example...</a>' - - expect(helper.event_note(input)).to match(link_url) - expect(helper.event_note(input)).to match(expected_link_text) - end - - it 'preserves code color scheme' do - input = "```ruby\ndef test\n 'hello world'\nend\n```" - expected = "\n<pre class=\"code highlight js-syntax-highlight ruby\">" \ - "<code><span class=\"line\"><span class=\"k\">def</span> <span class=\"nf\">test</span>...</span>\n" \ - "</code></pre>" - expect(helper.event_note(input)).to eq(expected) - end - - it 'preserves data-src for lazy images' do - input = "![ImageTest](/uploads/test.png)" - image_url = "data-src=\"/uploads/test.png\"" - expect(helper.event_note(input)).to match(image_url) - end - - context 'labels formatting' do - let(:input) { 'this should be ~label_1' } - - def format_event_note(project) - create(:label, title: 'label_1', project: project) - - helper.event_note(input, { project: project }) - end - - it 'preserves style attribute for a label that can be accessed by current_user' do - project = create(:project, :public) - - expect(format_event_note(project)).to match(/span class=.*style=.*/) - end - - it 'does not style a label that can not be accessed by current_user' do - project = create(:project, :private) - - expect(format_event_note(project)).to eq("<p>#{input}</p>") - end - end - end - describe '#event_commit_title' do let(:message) { "foo & bar " + "A" * 70 + "\n" + "B" * 80 } subject { helper.event_commit_title(message) } diff --git a/spec/helpers/markup_helper_spec.rb b/spec/helpers/markup_helper_spec.rb index 03d706062b7..62ea6d48542 100644 --- a/spec/helpers/markup_helper_spec.rb +++ b/spec/helpers/markup_helper_spec.rb @@ -67,7 +67,7 @@ describe MarkupHelper do describe 'without redacted attribute' do it 'renders the markdown value' do - expect(Banzai).to receive(:render_field).with(commit, attribute).and_call_original + expect(Banzai).to receive(:render_field).with(commit, attribute, {}).and_call_original helper.markdown_field(commit, attribute) end @@ -252,38 +252,141 @@ describe MarkupHelper do end describe '#first_line_in_markdown' do - it 'truncates Markdown properly' do - text = "@#{user.username}, can you look at this?\nHello world\n" - actual = first_line_in_markdown(text, 100, project: project) + shared_examples_for 'common markdown examples' do + let(:project_base) { build(:project, :repository) } - doc = Nokogiri::HTML.parse(actual) + it 'displays inline code' do + object = create_object('Text with `inline code`') + expected = 'Text with <code>inline code</code>' - # Make sure we didn't create invalid markup - expect(doc.errors).to be_empty + expect(first_line_in_markdown(object, attribute, 100, project: project)).to match(expected) + end - # Leading user link - expect(doc.css('a').length).to eq(1) - expect(doc.css('a')[0].attr('href')).to eq user_path(user) - expect(doc.css('a')[0].text).to eq "@#{user.username}" + it 'truncates the text with multiple paragraphs' do + object = create_object("Paragraph 1\n\nParagraph 2") + expected = 'Paragraph 1...' - expect(doc.content).to eq "@#{user.username}, can you look at this?..." - end + expect(first_line_in_markdown(object, attribute, 100, project: project)).to match(expected) + end - it 'truncates Markdown with emoji properly' do - text = "foo :wink:\nbar :grinning:" - actual = first_line_in_markdown(text, 100, project: project) + it 'displays the first line of a code block' do + object = create_object("```\nCode block\nwith two lines\n```") + expected = %r{<pre.+><code><span class="line">Code block\.\.\.</span>\n</code></pre>} - doc = Nokogiri::HTML.parse(actual) + expect(first_line_in_markdown(object, attribute, 100, project: project)).to match(expected) + end - # Make sure we didn't create invalid markup - # But also account for the 2 errors caused by the unknown `gl-emoji` elements - expect(doc.errors.length).to eq(2) + it 'truncates a single long line of text' do + text = 'The quick brown fox jumped over the lazy dog twice' # 50 chars + object = create_object(text * 4) + expected = (text * 2).sub(/.{3}/, '...') + + expect(first_line_in_markdown(object, attribute, 150, project: project)).to match(expected) + end + + it 'preserves a link href when link text is truncated' do + text = 'The quick brown fox jumped over the lazy dog' # 44 chars + input = "#{text}#{text}#{text} " # 133 chars + link_url = 'http://example.com/foo/bar/baz' # 30 chars + input << link_url + object = create_object(input) + expected_link_text = 'http://example...</a>' + + expect(first_line_in_markdown(object, attribute, 150, project: project)).to match(link_url) + expect(first_line_in_markdown(object, attribute, 150, project: project)).to match(expected_link_text) + end + + it 'preserves code color scheme' do + object = create_object("```ruby\ndef test\n 'hello world'\nend\n```") + expected = "\n<pre class=\"code highlight js-syntax-highlight ruby\">" \ + "<code><span class=\"line\"><span class=\"k\">def</span> <span class=\"nf\">test</span>...</span>\n" \ + "</code></pre>" + + expect(first_line_in_markdown(object, attribute, 150, project: project)).to eq(expected) + end + + it 'preserves data-src for lazy images' do + object = create_object("![ImageTest](/uploads/test.png)") + image_url = "data-src=\".*/uploads/test.png\"" + + expect(first_line_in_markdown(object, attribute, 150, project: project)).to match(image_url) + end + + context 'labels formatting' do + let(:label_title) { 'this should be ~label_1' } + + def create_and_format_label(project) + create(:label, title: 'label_1', project: project) + object = create_object(label_title, project: project) - expect(doc.css('gl-emoji').length).to eq(2) - expect(doc.css('gl-emoji')[0].attr('data-name')).to eq 'wink' - expect(doc.css('gl-emoji')[1].attr('data-name')).to eq 'grinning' + first_line_in_markdown(object, attribute, 150, project: project) + end - expect(doc.content).to eq "foo 😉\nbar 😀" + it 'preserves style attribute for a label that can be accessed by current_user' do + project = create(:project, :public) + + expect(create_and_format_label(project)).to match(/span class=.*style=.*/) + end + + it 'does not style a label that can not be accessed by current_user' do + project = create(:project, :private) + + expect(create_and_format_label(project)).to eq("<p>#{label_title}</p>") + end + end + + it 'truncates Markdown properly' do + object = create_object("@#{user.username}, can you look at this?\nHello world\n") + actual = first_line_in_markdown(object, attribute, 100, project: project) + + doc = Nokogiri::HTML.parse(actual) + + # Make sure we didn't create invalid markup + expect(doc.errors).to be_empty + + # Leading user link + expect(doc.css('a').length).to eq(1) + expect(doc.css('a')[0].attr('href')).to eq user_path(user) + expect(doc.css('a')[0].text).to eq "@#{user.username}" + + expect(doc.content).to eq "@#{user.username}, can you look at this?..." + end + + it 'truncates Markdown with emoji properly' do + object = create_object("foo :wink:\nbar :grinning:") + actual = first_line_in_markdown(object, attribute, 100, project: project) + + doc = Nokogiri::HTML.parse(actual) + + # Make sure we didn't create invalid markup + # But also account for the 2 errors caused by the unknown `gl-emoji` elements + expect(doc.errors.length).to eq(2) + + expect(doc.css('gl-emoji').length).to eq(2) + expect(doc.css('gl-emoji')[0].attr('data-name')).to eq 'wink' + expect(doc.css('gl-emoji')[1].attr('data-name')).to eq 'grinning' + + expect(doc.content).to eq "foo 😉\nbar 😀" + end + end + + context 'when the asked attribute can be redacted' do + include_examples 'common markdown examples' do + let(:attribute) { :note } + def create_object(title, project: project_base) + build(:note, note: title, project: project) + end + end + end + + context 'when the asked attribute can not be redacted' do + include_examples 'common markdown examples' do + let(:attribute) { :body } + def create_object(title, project: project_base) + issue = build(:issue, title: title) + build(:todo, :done, project: project_base, author: user, target: issue) + end + end end end diff --git a/spec/initializers/8_metrics_spec.rb b/spec/initializers/8_metrics_spec.rb index 4e6052a9f80..80c77057065 100644 --- a/spec/initializers/8_metrics_spec.rb +++ b/spec/initializers/8_metrics_spec.rb @@ -3,7 +3,6 @@ require 'spec_helper' describe 'instrument_classes' do let(:config) { double(:config) } - let(:unicorn_sampler) { double(:unicorn_sampler) } let(:influx_sampler) { double(:influx_sampler) } before do @@ -11,9 +10,7 @@ describe 'instrument_classes' do allow(config).to receive(:instrument_methods) allow(config).to receive(:instrument_instance_method) allow(config).to receive(:instrument_instance_methods) - allow(Gitlab::Metrics::UnicornSampler).to receive(:initialize_instance).and_return(unicorn_sampler) - allow(Gitlab::Metrics::InfluxSampler).to receive(:initialize_instance).and_return(influx_sampler) - allow(unicorn_sampler).to receive(:start) + allow(Gitlab::Metrics::Samplers::InfluxSampler).to receive(:initialize_instance).and_return(influx_sampler) allow(influx_sampler).to receive(:start) allow(Gitlab::Application).to receive(:configure) end diff --git a/spec/javascripts/behaviors/copy_as_gfm_spec.js b/spec/javascripts/behaviors/copy_as_gfm_spec.js new file mode 100644 index 00000000000..b8155144e2a --- /dev/null +++ b/spec/javascripts/behaviors/copy_as_gfm_spec.js @@ -0,0 +1,47 @@ +import { CopyAsGFM } from '~/behaviors/copy_as_gfm'; + +describe('CopyAsGFM', () => { + describe('CopyAsGFM.pasteGFM', () => { + function callPasteGFM() { + const e = { + originalEvent: { + clipboardData: { + getData(mimeType) { + // When GFM code is copied, we put the regular plain text + // on the clipboard as `text/plain`, and the GFM as `text/x-gfm`. + // This emulates the behavior of `getData` with that data. + if (mimeType === 'text/plain') { + return 'code'; + } + if (mimeType === 'text/x-gfm') { + return '`code`'; + } + return null; + }, + }, + }, + preventDefault() {}, + }; + + CopyAsGFM.pasteGFM(e); + } + + it('wraps pasted code when not already in code tags', () => { + spyOn(window.gl.utils, 'insertText').and.callFake((el, textFunc) => { + const insertedText = textFunc('This is code: ', ''); + expect(insertedText).toEqual('`code`'); + }); + + callPasteGFM(); + }); + + it('does not wrap pasted code when already in code tags', () => { + spyOn(window.gl.utils, 'insertText').and.callFake((el, textFunc) => { + const insertedText = textFunc('This is code: `', '`'); + expect(insertedText).toEqual('code'); + }); + + callPasteGFM(); + }); + }); +}); diff --git a/spec/javascripts/copy_as_gfm_spec.js b/spec/javascripts/copy_as_gfm_spec.js deleted file mode 100644 index ded450749d3..00000000000 --- a/spec/javascripts/copy_as_gfm_spec.js +++ /dev/null @@ -1,49 +0,0 @@ -import '~/copy_as_gfm'; - -(() => { - describe('gl.CopyAsGFM', () => { - describe('gl.CopyAsGFM.pasteGFM', () => { - function callPasteGFM() { - const e = { - originalEvent: { - clipboardData: { - getData(mimeType) { - // When GFM code is copied, we put the regular plain text - // on the clipboard as `text/plain`, and the GFM as `text/x-gfm`. - // This emulates the behavior of `getData` with that data. - if (mimeType === 'text/plain') { - return 'code'; - } - if (mimeType === 'text/x-gfm') { - return '`code`'; - } - return null; - }, - }, - }, - preventDefault() {}, - }; - - window.gl.CopyAsGFM.pasteGFM(e); - } - - it('wraps pasted code when not already in code tags', () => { - spyOn(window.gl.utils, 'insertText').and.callFake((el, textFunc) => { - const insertedText = textFunc('This is code: ', ''); - expect(insertedText).toEqual('`code`'); - }); - - callPasteGFM(); - }); - - it('does not wrap pasted code when already in code tags', () => { - spyOn(window.gl.utils, 'insertText').and.callFake((el, textFunc) => { - const insertedText = textFunc('This is code: `', '`'); - expect(insertedText).toEqual('code'); - }); - - callPasteGFM(); - }); - }); - }); -})(); diff --git a/spec/javascripts/fixtures/clusters.rb b/spec/javascripts/fixtures/clusters.rb index 5774f36f026..8e74c4f859c 100644 --- a/spec/javascripts/fixtures/clusters.rb +++ b/spec/javascripts/fixtures/clusters.rb @@ -6,7 +6,7 @@ describe Projects::ClustersController, '(JavaScript fixtures)', type: :controlle let(:admin) { create(:admin) } let(:namespace) { create(:namespace, name: 'frontend-fixtures' )} let(:project) { create(:project, :repository, namespace: namespace) } - let(:cluster) { project.create_cluster!(gcp_cluster_name: "gke-test-creation-1", gcp_project_id: 'gitlab-internal-153318', gcp_cluster_zone: 'us-central1-a', gcp_cluster_size: '1', project_namespace: 'aaa', gcp_machine_type: 'n1-standard-1')} + let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } render_views diff --git a/spec/javascripts/fixtures/search_autocomplete.html.haml b/spec/javascripts/fixtures/search_autocomplete.html.haml index 7785120da5b..0421ed2182f 100644 --- a/spec/javascripts/fixtures/search_autocomplete.html.haml +++ b/spec/javascripts/fixtures/search_autocomplete.html.haml @@ -8,3 +8,4 @@ %input#search.search-input.dropdown-menu-toggle .dropdown-menu.dropdown-select .dropdown-content + %input{ type: "hidden", class: "js-search-project-options" } diff --git a/spec/javascripts/lib/utils/datefix_spec.js b/spec/javascripts/lib/utils/datefix_spec.js index 0b9fde2be67..e58ac4300ba 100644 --- a/spec/javascripts/lib/utils/datefix_spec.js +++ b/spec/javascripts/lib/utils/datefix_spec.js @@ -1,4 +1,4 @@ -import { pad, parsePikadayDate, pikadayToString } from '~/lib/utils/datefix'; +import { pad, pikadayToString } from '~/lib/utils/datefix'; describe('datefix', () => { describe('pad', () => { @@ -16,9 +16,7 @@ describe('datefix', () => { }); describe('parsePikadayDate', () => { - it('should return a UTC date', () => { - expect(parsePikadayDate('2020-01-29')).toEqual(new Date('2020-01-29')); - }); + // removed because of https://gitlab.com/gitlab-org/gitlab-ce/issues/39834 }); describe('pikadayToString', () => { diff --git a/spec/javascripts/monitoring/graph/legend_spec.js b/spec/javascripts/monitoring/graph/legend_spec.js index 2571b7ef869..145c8db28d5 100644 --- a/spec/javascripts/monitoring/graph/legend_spec.js +++ b/spec/javascripts/monitoring/graph/legend_spec.js @@ -28,7 +28,7 @@ const defaultValuesComponent = { currentDataIndex: 0, }; -const timeSeries = createTimeSeries(convertedMetrics[0].queries[0], +const timeSeries = createTimeSeries(convertedMetrics[0].queries, defaultValuesComponent.graphWidth, defaultValuesComponent.graphHeight, defaultValuesComponent.graphHeightOffset); diff --git a/spec/javascripts/monitoring/graph_path_spec.js b/spec/javascripts/monitoring/graph_path_spec.js index 81825a3ae87..8ece913ada8 100644 --- a/spec/javascripts/monitoring/graph_path_spec.js +++ b/spec/javascripts/monitoring/graph_path_spec.js @@ -13,7 +13,7 @@ const createComponent = (propsData) => { const convertedMetrics = convertDatesMultipleSeries(singleRowMetricsMultipleSeries); -const timeSeries = createTimeSeries(convertedMetrics[0].queries[0], 428, 272, 120); +const timeSeries = createTimeSeries(convertedMetrics[0].queries, 428, 272, 120); const firstTimeSeries = timeSeries[0]; describe('Monitoring Paths', () => { diff --git a/spec/javascripts/monitoring/utils/multiple_time_series_spec.js b/spec/javascripts/monitoring/utils/multiple_time_series_spec.js index 7e44a9ade9e..99584c75287 100644 --- a/spec/javascripts/monitoring/utils/multiple_time_series_spec.js +++ b/spec/javascripts/monitoring/utils/multiple_time_series_spec.js @@ -2,7 +2,7 @@ import createTimeSeries from '~/monitoring/utils/multiple_time_series'; import { convertDatesMultipleSeries, singleRowMetricsMultipleSeries } from '../mock_data'; const convertedMetrics = convertDatesMultipleSeries(singleRowMetricsMultipleSeries); -const timeSeries = createTimeSeries(convertedMetrics[0].queries[0], 428, 272, 120); +const timeSeries = createTimeSeries(convertedMetrics[0].queries, 428, 272, 120); const firstTimeSeries = timeSeries[0]; describe('Multiple time series', () => { diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index 53d8faae911..928a4b461cc 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -343,6 +343,7 @@ import '~/notes'; diff_discussion_html: false, }; $form = jasmine.createSpyObj('$form', ['closest', 'find']); + $form.length = 1; row = jasmine.createSpyObj('row', ['prevAll', 'first', 'find']); notes = jasmine.createSpyObj('notes', [ @@ -371,13 +372,29 @@ import '~/notes'; $form.closest.and.returnValues(row, $form); $form.find.and.returnValues(discussionContainer); body.attr.and.returnValue(''); - - Notes.prototype.renderDiscussionNote.call(notes, note, $form); }); it('should call Notes.animateAppendNote', () => { + Notes.prototype.renderDiscussionNote.call(notes, note, $form); + expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.discussion_html, $('.main-notes-list')); }); + + it('should append to row selected with line_code', () => { + $form.length = 0; + note.discussion_line_code = 'line_code'; + note.diff_discussion_html = '<tr></tr>'; + + const line = document.createElement('div'); + line.id = note.discussion_line_code; + document.body.appendChild(line); + + $form.closest.and.returnValues($form); + + Notes.prototype.renderDiscussionNote.call(notes, note, $form); + + expect(line.nextSibling.outerHTML).toEqual(note.diff_discussion_html); + }); }); describe('Discussion sub note', () => { diff --git a/spec/javascripts/search_autocomplete_spec.js b/spec/javascripts/search_autocomplete_spec.js index 5e55a5d2686..a2394857b82 100644 --- a/spec/javascripts/search_autocomplete_spec.js +++ b/spec/javascripts/search_autocomplete_spec.js @@ -57,6 +57,10 @@ import '~/lib/utils/common_utils'; } }; + const disableProjectIssues = function() { + document.querySelector('.js-search-project-options').setAttribute('data-issues-disabled', true); + }; + // Mock `gl` object in window for dashboard specific page. App code will need it. mockDashboardOptions = function() { window.gl || (window.gl = {}); @@ -91,18 +95,20 @@ import '~/lib/utils/common_utils'; assertLinks = function(list, issuesPath, mrsPath) { var a1, a2, a3, a4, issuesAssignedToMeLink, issuesIHaveCreatedLink, mrsAssignedToMeLink, mrsIHaveCreatedLink; - issuesAssignedToMeLink = issuesPath + "/?assignee_username=" + userName; - issuesIHaveCreatedLink = issuesPath + "/?author_username=" + userName; + if (issuesPath) { + issuesAssignedToMeLink = issuesPath + "/?assignee_username=" + userName; + issuesIHaveCreatedLink = issuesPath + "/?author_username=" + userName; + a1 = "a[href='" + issuesAssignedToMeLink + "']"; + a2 = "a[href='" + issuesIHaveCreatedLink + "']"; + expect(list.find(a1).length).toBe(1); + expect(list.find(a1).text()).toBe('Issues assigned to me'); + expect(list.find(a2).length).toBe(1); + expect(list.find(a2).text()).toBe("Issues I've created"); + } mrsAssignedToMeLink = mrsPath + "/?assignee_username=" + userName; mrsIHaveCreatedLink = mrsPath + "/?author_username=" + userName; - a1 = "a[href='" + issuesAssignedToMeLink + "']"; - a2 = "a[href='" + issuesIHaveCreatedLink + "']"; a3 = "a[href='" + mrsAssignedToMeLink + "']"; a4 = "a[href='" + mrsIHaveCreatedLink + "']"; - expect(list.find(a1).length).toBe(1); - expect(list.find(a1).text()).toBe('Issues assigned to me'); - expect(list.find(a2).length).toBe(1); - expect(list.find(a2).text()).toBe("Issues I've created"); expect(list.find(a3).length).toBe(1); expect(list.find(a3).text()).toBe('Merge requests assigned to me'); expect(list.find(a4).length).toBe(1); @@ -153,6 +159,14 @@ import '~/lib/utils/common_utils'; list = widget.wrap.find('.dropdown-menu').find('ul'); return assertLinks(list, projectIssuesPath, projectMRsPath); }); + it('should show only Project mergeRequest dropdown menu items when project issues are disabled', function() { + addBodyAttributes('project'); + disableProjectIssues(); + mockProjectOptions(); + widget.searchInput.triggerHandler('focus'); + const list = widget.wrap.find('.dropdown-menu').find('ul'); + assertLinks(list, null, projectMRsPath); + }); it('should not show category related menu if there is text in the input', function() { var link, list; addBodyAttributes('project'); diff --git a/spec/javascripts/shortcuts_issuable_spec.js b/spec/javascripts/shortcuts_issuable_spec.js index f6320db8dc4..5d6a885d4cc 100644 --- a/spec/javascripts/shortcuts_issuable_spec.js +++ b/spec/javascripts/shortcuts_issuable_spec.js @@ -1,6 +1,8 @@ -import '~/copy_as_gfm'; +import initCopyAsGFM from '~/behaviors/copy_as_gfm'; import ShortcutsIssuable from '~/shortcuts_issuable'; +initCopyAsGFM(); + describe('ShortcutsIssuable', () => { const fixtureName = 'merge_requests/diff_comment.html.raw'; preloadFixtures(fixtureName); diff --git a/spec/javascripts/test_bundle.js b/spec/javascripts/test_bundle.js index d4e134583c7..fd7aa332d17 100644 --- a/spec/javascripts/test_bundle.js +++ b/spec/javascripts/test_bundle.js @@ -11,6 +11,12 @@ const isHeadlessChrome = /\bHeadlessChrome\//.test(navigator.userAgent); Vue.config.devtools = !isHeadlessChrome; Vue.config.productionTip = false; +let hasVueWarnings = false; +Vue.config.warnHandler = (msg, vm, trace) => { + hasVueWarnings = true; + fail(`${msg}${trace}`); +}; + Vue.use(VueResource); // enable test fixtures @@ -34,11 +40,6 @@ window.addEventListener('unhandledrejection', (event) => { console.error(event.reason.stack || event.reason); }); -const checkUnhandledPromiseRejections = (done) => { - expect(hasUnhandledPromiseRejections).toBe(false); - done(); -}; - // HACK: Chrome 59 disconnects if there are too many synchronous tests in a row // because it appears to lock up the thread that communicates to Karma's socket // This async beforeEach gets called on every spec and releases the JS thread long @@ -47,17 +48,6 @@ const checkUnhandledPromiseRejections = (done) => { // to run our unit tests. beforeEach(done => done()); -beforeAll(() => { - const origError = console.error; - spyOn(console, 'error').and.callFake((message) => { - if (/^\[Vue warn\]/.test(message)) { - fail(message); - } else { - origError(message); - } - }); -}); - const builtinVueHttpInterceptors = Vue.http.interceptors.slice(); beforeEach(() => { @@ -80,8 +70,22 @@ testsContext.keys().forEach(function (path) { } }); -it('has no unhandled Promise rejections', (done) => { - setTimeout(checkUnhandledPromiseRejections(done), 1000); +describe('test errors', () => { + beforeAll((done) => { + if (hasUnhandledPromiseRejections || hasVueWarnings) { + setTimeout(done, 1000); + } else { + done(); + } + }); + + it('has no unhandled Promise rejections', () => { + expect(hasUnhandledPromiseRejections).toBe(false); + }); + + it('has no Vue warnings', () => { + expect(hasVueWarnings).toBe(false); + }); }); // if we're generating coverage reports, make sure to include all files so diff --git a/spec/javascripts/vue_shared/components/markdown/field_spec.js b/spec/javascripts/vue_shared/components/markdown/field_spec.js index 65c49b9f30b..24209be83fe 100644 --- a/spec/javascripts/vue_shared/components/markdown/field_spec.js +++ b/spec/javascripts/vue_shared/components/markdown/field_spec.js @@ -1,6 +1,12 @@ import Vue from 'vue'; import fieldComponent from '~/vue_shared/components/markdown/field.vue'; +function assertMarkdownTabs(isWrite, writeLink, previewLink, vm) { + expect(writeLink.parentNode.classList.contains('active')).toEqual(isWrite); + expect(previewLink.parentNode.classList.contains('active')).toEqual(!isWrite); + expect(vm.$el.querySelector('.md-preview').style.display).toEqual(isWrite ? 'none' : ''); +} + describe('Markdown field component', () => { let vm; @@ -39,6 +45,7 @@ describe('Markdown field component', () => { describe('markdown preview', () => { let previewLink; + let writeLink; beforeEach(() => { spyOn(Vue.http, 'post').and.callFake(() => new Promise((resolve) => { @@ -53,7 +60,8 @@ describe('Markdown field component', () => { }); })); - previewLink = vm.$el.querySelector('.nav-links li:nth-child(2) a'); + previewLink = vm.$el.querySelector('.nav-links .js-preview-link'); + writeLink = vm.$el.querySelector('.nav-links .js-write-link'); }); it('sets preview link as active', (done) => { @@ -105,6 +113,23 @@ describe('Markdown field component', () => { done(); }, 0); }); + + it('clicking already active write or preview link does nothing', (done) => { + writeLink.click(); + Vue.nextTick() + .then(() => assertMarkdownTabs(true, writeLink, previewLink, vm)) + .then(() => writeLink.click()) + .then(() => Vue.nextTick()) + .then(() => assertMarkdownTabs(true, writeLink, previewLink, vm)) + .then(() => previewLink.click()) + .then(() => Vue.nextTick()) + .then(() => assertMarkdownTabs(false, writeLink, previewLink, vm)) + .then(() => previewLink.click()) + .then(() => Vue.nextTick()) + .then(() => assertMarkdownTabs(false, writeLink, previewLink, vm)) + .then(done) + .catch(done.fail); + }); }); describe('markdown buttons', () => { diff --git a/spec/javascripts/vue_shared/components/markdown/header_spec.js b/spec/javascripts/vue_shared/components/markdown/header_spec.js index 7110ff36937..edebd822295 100644 --- a/spec/javascripts/vue_shared/components/markdown/header_spec.js +++ b/spec/javascripts/vue_shared/components/markdown/header_spec.js @@ -43,11 +43,13 @@ describe('Markdown field header component', () => { it('emits toggle markdown event when clicking preview', () => { spyOn(vm, '$emit'); - vm.$el.querySelector('li:nth-child(2) a').click(); + vm.$el.querySelector('.js-preview-link').click(); - expect( - vm.$emit, - ).toHaveBeenCalledWith('toggle-markdown'); + expect(vm.$emit).toHaveBeenCalledWith('preview-markdown'); + + vm.$el.querySelector('.js-write-link').click(); + + expect(vm.$emit).toHaveBeenCalledWith('write-markdown'); }); it('blurs preview link after click', (done) => { diff --git a/spec/lib/banzai/commit_renderer_spec.rb b/spec/lib/banzai/commit_renderer_spec.rb index 049d025a5b9..84adaebdcbe 100644 --- a/spec/lib/banzai/commit_renderer_spec.rb +++ b/spec/lib/banzai/commit_renderer_spec.rb @@ -10,7 +10,7 @@ describe Banzai::CommitRenderer do described_class::ATTRIBUTES.each do |attr| expect_any_instance_of(Banzai::ObjectRenderer).to receive(:render).with([project.commit], attr).once.and_call_original - expect(Banzai::Renderer).to receive(:cacheless_render_field).with(project.commit, attr) + expect(Banzai::Renderer).to receive(:cacheless_render_field).with(project.commit, attr, {}) end described_class.render([project.commit], project, user) diff --git a/spec/lib/banzai/filter/absolute_link_filter_spec.rb b/spec/lib/banzai/filter/absolute_link_filter_spec.rb new file mode 100644 index 00000000000..a3ad056efcd --- /dev/null +++ b/spec/lib/banzai/filter/absolute_link_filter_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe Banzai::Filter::AbsoluteLinkFilter do + def filter(doc, context = {}) + described_class.call(doc, context) + end + + context 'with html links' do + context 'if only_path is false' do + let(:only_path_context) do + { only_path: false } + end + let(:fake_url) { 'http://www.example.com' } + + before do + allow(Gitlab.config.gitlab).to receive(:url).and_return(fake_url) + end + + context 'has the .gfm class' do + it 'converts a relative url into absolute' do + doc = filter(link('/foo', 'gfm'), only_path_context) + expect(doc.at_css('a')['href']).to eq "#{fake_url}/foo" + end + + it 'does not change the url if it already absolute' do + doc = filter(link("#{fake_url}/foo", 'gfm'), only_path_context) + expect(doc.at_css('a')['href']).to eq "#{fake_url}/foo" + end + + context 'if relative_url_root is set' do + it 'joins the url without without doubling the path' do + allow(Gitlab.config.gitlab).to receive(:url).and_return("#{fake_url}/gitlab/") + doc = filter(link("/gitlab/foo", 'gfm'), only_path_context) + expect(doc.at_css('a')['href']).to eq "#{fake_url}/gitlab/foo" + end + end + end + + context 'has not the .gfm class' do + it 'does not convert a relative url into absolute' do + doc = filter(link('/foo'), only_path_context) + expect(doc.at_css('a')['href']).to eq '/foo' + end + end + end + + context 'if only_path is not false' do + it 'does not convert a relative url into absolute' do + expect(filter(link('/foo', 'gfm')).at_css('a')['href']).to eq '/foo' + expect(filter(link('/foo')).at_css('a')['href']).to eq '/foo' + end + end + end + + def link(path, css_class = '') + %(<a class="#{css_class}" href="#{path}">example</a>) + end +end diff --git a/spec/lib/banzai/note_renderer_spec.rb b/spec/lib/banzai/note_renderer_spec.rb deleted file mode 100644 index 32764bee5eb..00000000000 --- a/spec/lib/banzai/note_renderer_spec.rb +++ /dev/null @@ -1,24 +0,0 @@ -require 'spec_helper' - -describe Banzai::NoteRenderer do - describe '.render' do - it 'renders a Note' do - note = double(:note) - project = double(:project) - wiki = double(:wiki) - user = double(:user) - - expect(Banzai::ObjectRenderer).to receive(:new) - .with(project, user, - requested_path: 'foo', - project_wiki: wiki, - ref: 'bar') - .and_call_original - - expect_any_instance_of(Banzai::ObjectRenderer) - .to receive(:render).with([note], :note) - - described_class.render([note], project, user, 'foo', wiki, 'bar') - end - end -end diff --git a/spec/lib/banzai/object_renderer_spec.rb b/spec/lib/banzai/object_renderer_spec.rb index b172a1b718c..074d521a5c6 100644 --- a/spec/lib/banzai/object_renderer_spec.rb +++ b/spec/lib/banzai/object_renderer_spec.rb @@ -22,7 +22,7 @@ describe Banzai::ObjectRenderer do end it 'retrieves field content using Banzai::Renderer.render_field' do - expect(Banzai::Renderer).to receive(:render_field).with(object, :note).and_call_original + expect(Banzai::Renderer).to receive(:render_field).with(object, :note, {}).and_call_original renderer.render([object], :note) end @@ -68,7 +68,7 @@ describe Banzai::ObjectRenderer do end it 'retrieves field content using Banzai::Renderer.cacheless_render_field' do - expect(Banzai::Renderer).to receive(:cacheless_render_field).with(commit, :title).and_call_original + expect(Banzai::Renderer).to receive(:cacheless_render_field).with(commit, :title, {}).and_call_original renderer.render([commit], :title) end diff --git a/spec/lib/banzai/renderer_spec.rb b/spec/lib/banzai/renderer_spec.rb index 81a04a2d46d..650cecfc778 100644 --- a/spec/lib/banzai/renderer_spec.rb +++ b/spec/lib/banzai/renderer_spec.rb @@ -18,7 +18,7 @@ describe Banzai::Renderer do let(:commit) { create(:project, :repository).commit } it 'returns cacheless render field' do - expect(renderer).to receive(:cacheless_render_field).with(commit, :title) + expect(renderer).to receive(:cacheless_render_field).with(commit, :title, {}) renderer.render_field(commit, :title) end diff --git a/spec/lib/gitlab/checks/change_access_spec.rb b/spec/lib/gitlab/checks/change_access_spec.rb index 6c25b7349e1..74a24a4424b 100644 --- a/spec/lib/gitlab/checks/change_access_spec.rb +++ b/spec/lib/gitlab/checks/change_access_spec.rb @@ -11,13 +11,13 @@ describe Gitlab::Checks::ChangeAccess do let(:changes) { { oldrev: oldrev, newrev: newrev, ref: ref } } let(:protocol) { 'ssh' } - subject do + subject(:change_access) do described_class.new( changes, project: project, user_access: user_access, protocol: protocol - ).exec + ) end before do @@ -26,7 +26,7 @@ describe Gitlab::Checks::ChangeAccess do context 'without failed checks' do it "doesn't raise an error" do - expect { subject }.not_to raise_error + expect { subject.exec }.not_to raise_error end end @@ -34,7 +34,7 @@ describe Gitlab::Checks::ChangeAccess do it 'raises an error' do expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false) - expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to this project.') + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to this project.') end end @@ -45,7 +45,7 @@ describe Gitlab::Checks::ChangeAccess do allow(user_access).to receive(:can_do_action?).with(:push_code).and_return(true) expect(user_access).to receive(:can_do_action?).with(:admin_project).and_return(false) - expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to change existing tags on this project.') + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to change existing tags on this project.') end context 'with protected tag' do @@ -61,7 +61,7 @@ describe Gitlab::Checks::ChangeAccess do let(:newrev) { '0000000000000000000000000000000000000000' } it 'is prevented' do - expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /cannot be deleted/) + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /cannot be deleted/) end end @@ -70,7 +70,7 @@ describe Gitlab::Checks::ChangeAccess do let(:newrev) { '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51' } it 'is prevented' do - expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /cannot be updated/) + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /cannot be updated/) end end end @@ -81,14 +81,14 @@ describe Gitlab::Checks::ChangeAccess do let(:ref) { 'refs/tags/v9.1.0' } it 'prevents creation below access level' do - expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /allowed to create this tag as it is protected/) + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /allowed to create this tag as it is protected/) end context 'when user has access' do let!(:protected_tag) { create(:protected_tag, :developers_can_create, project: project, name: 'v*') } it 'allows tag creation' do - expect { subject }.not_to raise_error + expect { subject.exec }.not_to raise_error end end end @@ -101,7 +101,7 @@ describe Gitlab::Checks::ChangeAccess do let(:ref) { 'refs/heads/master' } it 'raises an error' do - expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'The default branch of a project cannot be deleted.') + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'The default branch of a project cannot be deleted.') end end @@ -114,7 +114,7 @@ describe Gitlab::Checks::ChangeAccess do it 'raises an error if the user is not allowed to do forced pushes to protected branches' do expect(Gitlab::Checks::ForcePush).to receive(:force_push?).and_return(true) - expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to force push code to a protected branch on this project.') + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to force push code to a protected branch on this project.') end it 'raises an error if the user is not allowed to merge to protected branches' do @@ -122,13 +122,13 @@ describe Gitlab::Checks::ChangeAccess do expect(user_access).to receive(:can_merge_to_branch?).and_return(false) expect(user_access).to receive(:can_push_to_branch?).and_return(false) - expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to merge code into protected branches on this project.') + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to merge code into protected branches on this project.') end it 'raises an error if the user is not allowed to push to protected branches' do expect(user_access).to receive(:can_push_to_branch?).and_return(false) - expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to protected branches on this project.') + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to protected branches on this project.') end context 'branch deletion' do @@ -137,7 +137,7 @@ describe Gitlab::Checks::ChangeAccess do context 'if the user is not allowed to delete protected branches' do it 'raises an error' do - expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to delete protected branches from this project. Only a project master or owner can delete a protected branch.') + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to delete protected branches from this project. Only a project master or owner can delete a protected branch.') end end @@ -150,18 +150,63 @@ describe Gitlab::Checks::ChangeAccess do let(:protocol) { 'web' } it 'allows branch deletion' do - expect { subject }.not_to raise_error + expect { subject.exec }.not_to raise_error end end context 'over SSH or HTTP' do it 'raises an error' do - expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only delete protected branches using the web interface.') + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only delete protected branches using the web interface.') end end end end end end + + context 'LFS integrity check' do + let(:blob_object) { project.repository.blob_at_branch('lfs', 'files/lfs/lfs_object.iso') } + + before do + allow_any_instance_of(Gitlab::Git::RevList).to receive(:new_objects) do |&lazy_block| + lazy_block.call([blob_object.id]) + end + end + + context 'with LFS not enabled' do + it 'skips integrity check' do + expect_any_instance_of(Gitlab::Git::RevList).not_to receive(:new_objects) + + subject.exec + end + end + + context 'with LFS enabled' do + before do + allow(project).to receive(:lfs_enabled?).and_return(true) + end + + context 'deletion' do + let(:changes) { { oldrev: oldrev, ref: ref } } + + it 'skips integrity check' do + expect_any_instance_of(Gitlab::Git::RevList).not_to receive(:new_objects) + + subject.exec + end + end + + it 'fails if any LFS blobs are missing' do + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /LFS objects are missing/) + end + + it 'succeeds if LFS objects have already been uploaded' do + lfs_object = create(:lfs_object, oid: blob_object.lfs_oid) + create(:lfs_objects_project, project: project, lfs_object: lfs_object) + + expect { subject.exec }.not_to raise_error + end + end + end end end diff --git a/spec/lib/gitlab/ci/cron_parser_spec.rb b/spec/lib/gitlab/ci/cron_parser_spec.rb index 809fda11879..2a3f7807fdb 100644 --- a/spec/lib/gitlab/ci/cron_parser_spec.rb +++ b/spec/lib/gitlab/ci/cron_parser_spec.rb @@ -77,8 +77,20 @@ describe Gitlab::Ci::CronParser do it_behaves_like "returns time in the future" - it 'converts time in server time zone' do - expect(subject.hour).to eq(hour_in_utc) + context 'when PST (Pacific Standard Time)' do + it 'converts time in server time zone' do + Timecop.freeze(Time.utc(2017, 1, 1)) do + expect(subject.hour).to eq(hour_in_utc) + end + end + end + + context 'when PDT (Pacific Daylight Time)' do + it 'converts time in server time zone' do + Timecop.freeze(Time.utc(2017, 6, 1)) do + expect(subject.hour).to eq(hour_in_utc) + end + end end end end @@ -100,8 +112,20 @@ describe Gitlab::Ci::CronParser do it_behaves_like "returns time in the future" - it 'converts time in server time zone' do - expect(subject.hour).to eq(hour_in_utc) + context 'when CET (Central European Time)' do + it 'converts time in server time zone' do + Timecop.freeze(Time.utc(2017, 1, 1)) do + expect(subject.hour).to eq(hour_in_utc) + end + end + end + + context 'when CEST (Central European Summer Time)' do + it 'converts time in server time zone' do + Timecop.freeze(Time.utc(2017, 6, 1)) do + expect(subject.hour).to eq(hour_in_utc) + end + end end end @@ -111,8 +135,20 @@ describe Gitlab::Ci::CronParser do it_behaves_like "returns time in the future" - it 'converts time in server time zone' do - expect(subject.hour).to eq(hour_in_utc) + context 'when EST (Eastern Standard Time)' do + it 'converts time in server time zone' do + Timecop.freeze(Time.utc(2017, 1, 1)) do + expect(subject.hour).to eq(hour_in_utc) + end + end + end + + context 'when EDT (Eastern Daylight Time)' do + it 'converts time in server time zone' do + Timecop.freeze(Time.utc(2017, 6, 1)) do + expect(subject.hour).to eq(hour_in_utc) + end + end end end end diff --git a/spec/lib/gitlab/git/lfs_changes_spec.rb b/spec/lib/gitlab/git/lfs_changes_spec.rb index c2d2c6e1bc8..c9007d7d456 100644 --- a/spec/lib/gitlab/git/lfs_changes_spec.rb +++ b/spec/lib/gitlab/git/lfs_changes_spec.rb @@ -9,7 +9,7 @@ describe Gitlab::Git::LfsChanges do describe 'new_pointers' do before do - allow_any_instance_of(Gitlab::Git::RevList).to receive(:new_objects).and_return([blob_object_id]) + allow_any_instance_of(Gitlab::Git::RevList).to receive(:new_objects).and_yield([blob_object_id]) end it 'uses rev-list to find new objects' do @@ -38,7 +38,7 @@ describe Gitlab::Git::LfsChanges do it 'uses rev-list to find all objects' do rev_list = double allow(Gitlab::Git::RevList).to receive(:new).and_return(rev_list) - allow(rev_list).to receive(:all_objects).and_return([blob_object_id]) + allow(rev_list).to receive(:all_objects).and_yield([blob_object_id]) expect(Gitlab::Git::Blob).to receive(:batch_lfs_pointers).with(project.repository, [blob_object_id]) diff --git a/spec/lib/gitlab/git/popen_spec.rb b/spec/lib/gitlab/git/popen_spec.rb index 2b65bc1cf15..b033ede9062 100644 --- a/spec/lib/gitlab/git/popen_spec.rb +++ b/spec/lib/gitlab/git/popen_spec.rb @@ -53,6 +53,23 @@ describe 'Gitlab::Git::Popen' do it { expect(status).to be_zero } it { expect(output).to eq('hello') } end + + context 'with lazy block' do + it 'yields a lazy io' do + expect_lazy_io = lambda do |io| + expect(io).to be_a Enumerator::Lazy + expect(io.inspect).to include('#<IO:fd') + end + + klass.new.popen(%w[ls], path, lazy_block: expect_lazy_io) + end + + it "doesn't wait for process exit" do + Timeout.timeout(2) do + klass.new.popen(%w[yes], path, lazy_block: ->(io) {}) + end + end + end end context 'popen_with_timeout' do diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index bf6d199ebe2..96e162ac087 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -559,10 +559,10 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - describe "#remote_delete" do + describe "#remove_remote" do before(:all) do @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - @repo.remote_delete("expendable") + @repo.remove_remote("expendable") end it "should remove the remote" do @@ -575,14 +575,16 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - describe "#remote_add" do + describe "#remote_update" do before(:all) do @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - @repo.remote_add("new_remote", SeedHelper::GITLAB_GIT_TEST_REPO_URL) + @repo.remote_update("expendable", url: TEST_NORMAL_REPO_PATH) end it "should add the remote" do - expect(@repo.rugged.remotes.each_name.to_a).to include("new_remote") + expect(@repo.rugged.remotes["expendable"].url).to( + eq(TEST_NORMAL_REPO_PATH) + ) end after(:all) do @@ -591,21 +593,58 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - describe "#remote_update" do - before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - @repo.remote_update("expendable", url: TEST_NORMAL_REPO_PATH) + describe '#fetch_mirror' do + let(:new_repository) do + Gitlab::Git::Repository.new('default', 'my_project.git', '') end - it "should add the remote" do - expect(@repo.rugged.remotes["expendable"].url).to( - eq(TEST_NORMAL_REPO_PATH) - ) + subject { new_repository.fetch_mirror(repository.path) } + + before do + Gitlab::Shell.new.add_repository('default', 'my_project') end - after(:all) do - FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH) - ensure_seeds + after do + Gitlab::Shell.new.remove_repository(TestEnv.repos_path, 'my_project') + end + + it 'fetches a url as a mirror remote' do + subject + + expect(refs(new_repository.path)).to eq(refs(repository.path)) + end + + context 'with keep-around refs' do + let(:sha) { SeedRepo::Commit::ID } + let(:keep_around_ref) { "refs/keep-around/#{sha}" } + let(:tmp_ref) { "refs/tmp/#{SecureRandom.hex}" } + + before do + repository.rugged.references.create(keep_around_ref, sha, force: true) + repository.rugged.references.create(tmp_ref, sha, force: true) + end + + it 'includes the temporary and keep-around refs' do + subject + + expect(refs(new_repository.path)).to include(keep_around_ref) + expect(refs(new_repository.path)).to include(tmp_ref) + end + end + end + + describe '#remote_tags' do + let(:target_commit_id) { SeedRepo::Commit::ID } + + subject { repository.remote_tags('upstream') } + + it 'gets the remote tags' do + expect(repository).to receive(:list_remote_tags).with('upstream') + .and_return(["#{target_commit_id}\trefs/tags/v0.0.1\n"]) + + expect(subject.first).to be_an_instance_of(Gitlab::Git::Tag) + expect(subject.first.name).to eq('v0.0.1') + expect(subject.first.dereferenced_target.id).to eq(target_commit_id) end end @@ -1482,7 +1521,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - describe '#fetch_source_branch' do + describe '#fetch_source_branch!' do let(:local_ref) { 'refs/merge-requests/1/head' } context 'when the branch exists' do @@ -1491,11 +1530,11 @@ describe Gitlab::Git::Repository, seed_helper: true do it 'writes the ref' do expect(repository).to receive(:write_ref).with(local_ref, /\h{40}/) - repository.fetch_source_branch(repository, source_branch, local_ref) + repository.fetch_source_branch!(repository, source_branch, local_ref) end it 'returns true' do - expect(repository.fetch_source_branch(repository, source_branch, local_ref)).to eq(true) + expect(repository.fetch_source_branch!(repository, source_branch, local_ref)).to eq(true) end end @@ -1505,11 +1544,11 @@ describe Gitlab::Git::Repository, seed_helper: true do it 'does not write the ref' do expect(repository).not_to receive(:write_ref) - repository.fetch_source_branch(repository, source_branch, local_ref) + repository.fetch_source_branch!(repository, source_branch, local_ref) end it 'returns false' do - expect(repository.fetch_source_branch(repository, source_branch, local_ref)).to eq(false) + expect(repository.fetch_source_branch!(repository, source_branch, local_ref)).to eq(false) end end end @@ -1685,6 +1724,21 @@ describe Gitlab::Git::Repository, seed_helper: true do end end + describe '#fetch' do + let(:git_path) { Gitlab.config.git.bin_path } + let(:remote_name) { 'my_remote' } + + subject { repository.fetch(remote_name) } + + it 'fetches the remote and returns true if the command was successful' do + expect(repository).to receive(:popen) + .with(%W(#{git_path} fetch #{remote_name}), repository.path) + .and_return(['', 0]) + + expect(subject).to be(true) + end + end + def create_remote_branch(repository, remote_name, branch_name, source_branch_name) source_branch = repository.branches.find { |branch| branch.name == source_branch_name } rugged = repository.rugged @@ -1760,4 +1814,10 @@ describe Gitlab::Git::Repository, seed_helper: true do sha = Rugged::Commit.create(repo, options) repo.lookup(sha) end + + def refs(dir) + IO.popen(%W[git -C #{dir} for-each-ref], &:read).split("\n").map do |line| + line.split("\t").last + end + end end diff --git a/spec/lib/gitlab/git/rev_list_spec.rb b/spec/lib/gitlab/git/rev_list_spec.rb index 643a4b2d03e..eaf74951b0e 100644 --- a/spec/lib/gitlab/git/rev_list_spec.rb +++ b/spec/lib/gitlab/git/rev_list_spec.rb @@ -3,26 +3,44 @@ require 'spec_helper' describe Gitlab::Git::RevList do let(:project) { create(:project, :repository) } let(:rev_list) { described_class.new(newrev: 'newrev', path_to_repo: project.repository.path_to_repo) } + let(:env_hash) do + { + 'GIT_OBJECT_DIRECTORY' => 'foo', + 'GIT_ALTERNATE_OBJECT_DIRECTORIES' => 'bar' + } + end before do - allow(Gitlab::Git::Env).to receive(:all).and_return({ - GIT_OBJECT_DIRECTORY: 'foo', - GIT_ALTERNATE_OBJECT_DIRECTORIES: 'bar' - }) + allow(Gitlab::Git::Env).to receive(:all).and_return(env_hash.symbolize_keys) end - def stub_popen_rev_list(*additional_args, output:) - expect(rev_list).to receive(:popen).with([ + def args_for_popen(args_list) + [ Gitlab.config.git.bin_path, "--git-dir=#{project.repository.path_to_repo}", 'rev-list', - *additional_args - ], - nil, - { - 'GIT_OBJECT_DIRECTORY' => 'foo', - 'GIT_ALTERNATE_OBJECT_DIRECTORIES' => 'bar' - }).and_return([output, 0]) + *args_list + ] + end + + def stub_popen_rev_list(*additional_args, output:) + args = args_for_popen(additional_args) + + expect(rev_list).to receive(:popen).with(args, nil, env_hash) + .and_return([output, 0]) + end + + def stub_lazy_popen_rev_list(*additional_args, output:) + params = [ + args_for_popen(additional_args), + nil, + env_hash, + hash_including(lazy_block: anything) + ] + + expect(rev_list).to receive(:popen).with(*params) do |*_, lazy_block:| + lazy_block.call(output.split("\n").lazy) + end end context "#new_refs" do @@ -46,10 +64,22 @@ describe Gitlab::Git::RevList do expect(rev_list.new_objects(require_path: true)).to eq(%w[sha2]) end - it 'can return a lazy enumerator' do - stub_popen_rev_list('newrev', '--not', '--all', '--objects', output: "sha1\nsha2") + it 'can yield a lazy enumerator' do + stub_lazy_popen_rev_list('newrev', '--not', '--all', '--objects', output: "sha1\nsha2") + + rev_list.new_objects do |object_ids| + expect(object_ids).to be_a Enumerator::Lazy + end + end + + it 'returns the result of the block when given' do + stub_lazy_popen_rev_list('newrev', '--not', '--all', '--objects', output: "sha1\nsha2") + + objects = rev_list.new_objects do |object_ids| + object_ids.first + end - expect(rev_list.new_objects(lazy: true)).to be_a Enumerator::Lazy + expect(objects).to eq 'sha1' end it 'can accept list of references to exclude' do @@ -69,7 +99,7 @@ describe Gitlab::Git::RevList do it 'fetches list of all pushed objects using rev-list' do stub_popen_rev_list('--all', '--objects', output: "sha1\nsha2") - expect(rev_list.all_objects.force).to eq(%w[sha1 sha2]) + expect(rev_list.all_objects).to eq(%w[sha1 sha2]) end end diff --git a/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb b/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb index 92bf87bbad4..78475403f9e 100644 --- a/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb +++ b/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb @@ -26,7 +26,6 @@ describe Gitlab::HookData::MergeRequestBuilder do merge_user_id merge_when_pipeline_succeeds milestone_id - ref_fetched source_branch source_project_id state diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 6c6b9154a0a..96efdd0949b 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -148,9 +148,18 @@ deploy_keys: - deploy_keys_projects - projects cluster: -- project +- cluster_projects +- projects - user -- service +- provider_gcp +- platform_kubernetes +cluster_projects: +- projects +- clusters +provider_gcp: +- cluster +platform_kubernetes: +- cluster services: - project - service_hook @@ -182,6 +191,7 @@ project: - tags - chat_services - cluster +- cluster_project - creator - group - namespace diff --git a/spec/lib/gitlab/import_export/fork_spec.rb b/spec/lib/gitlab/import_export/fork_spec.rb index dd0ce0dae41..cfb15ee7e8b 100644 --- a/spec/lib/gitlab/import_export/fork_spec.rb +++ b/spec/lib/gitlab/import_export/fork_spec.rb @@ -46,7 +46,7 @@ describe 'forked project import' do end it 'can access the MR' do - project.merge_requests.first.ensure_ref_fetched + project.merge_requests.first.fetch_ref! expect(project.repository.ref_exists?('refs/merge-requests/1/head')).to be_truthy end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 89d30407077..4b79e9f18c6 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -313,30 +313,47 @@ Ci::PipelineSchedule: - deleted_at - created_at - updated_at -Gcp::Cluster: +Clusters::Cluster: - id -- project_id - user_id -- service_id - enabled +- name +- provider_type +- platform_type +- created_at +- updated_at +Clusters::Project: +- id +- project_id +- cluster_id +- created_at +- updated_at +Clusters::Providers::Gcp: +- id +- cluster_id - status - status_reason -- project_namespace +- gcp_project_id +- zone +- num_nodes +- machine_type +- operation_id - endpoint +- encrypted_access_token +- encrypted_access_token_iv +- created_at +- updated_at +Clusters::Platforms::Kubernetes: +- id +- cluster_id +- api_url - ca_cert -- encrypted_kubernetes_token -- encrypted_kubernetes_token_iv +- namespace - username - encrypted_password - encrypted_password_iv -- gcp_project_id -- gcp_cluster_zone -- gcp_cluster_name -- gcp_cluster_size -- gcp_machine_type -- gcp_operation_id -- encrypted_gcp_token -- encrypted_gcp_token_iv +- encrypted_token +- encrypted_token_iv - created_at - updated_at DeployKey: diff --git a/spec/lib/gitlab/metrics/background_transaction_spec.rb b/spec/lib/gitlab/metrics/background_transaction_spec.rb new file mode 100644 index 00000000000..96052b8dc2f --- /dev/null +++ b/spec/lib/gitlab/metrics/background_transaction_spec.rb @@ -0,0 +1,13 @@ +require 'spec_helper' + +describe Gitlab::Metrics::BackgroundTransaction do + let(:test_worker_class) { double(:class, name: 'TestWorker') } + + subject { described_class.new(test_worker_class) } + + describe '#action' do + it 'returns transaction action name' do + expect(subject.action).to eq('TestWorker#perform') + end + end +end diff --git a/spec/lib/gitlab/metrics/instrumentation_spec.rb b/spec/lib/gitlab/metrics/instrumentation_spec.rb index 4b19ee19103..977bc250049 100644 --- a/spec/lib/gitlab/metrics/instrumentation_spec.rb +++ b/spec/lib/gitlab/metrics/instrumentation_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' describe Gitlab::Metrics::Instrumentation do - let(:transaction) { Gitlab::Metrics::Transaction.new } + let(:env) { {} } + let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) } before do @dummy = Class.new do diff --git a/spec/lib/gitlab/metrics/method_call_spec.rb b/spec/lib/gitlab/metrics/method_call_spec.rb index a247f03b2da..f1e9e414e0d 100644 --- a/spec/lib/gitlab/metrics/method_call_spec.rb +++ b/spec/lib/gitlab/metrics/method_call_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' describe Gitlab::Metrics::MethodCall do - let(:method_call) { described_class.new('Foo#bar', 'foo') } + let(:transaction) { double(:transaction, labels: {}) } + let(:method_call) { described_class.new('Foo#bar', :Foo, '#bar', transaction) } describe '#measure' do it 'measures the performance of the supplied block' do @@ -11,6 +12,18 @@ describe Gitlab::Metrics::MethodCall do expect(method_call.cpu_time).to be_a_kind_of(Numeric) expect(method_call.call_count).to eq(1) end + + it 'observes the performance of the supplied block' do + expect(described_class.call_real_duration_histogram) + .to receive(:observe) + .with({ module: :Foo, method: '#bar' }, be_a_kind_of(Numeric)) + + expect(described_class.call_cpu_duration_histogram) + .to receive(:observe) + .with({ module: :Foo, method: '#bar' }, be_a_kind_of(Numeric)) + + method_call.measure { 'foo' } + end end describe '#to_metric' do @@ -19,7 +32,7 @@ describe Gitlab::Metrics::MethodCall do metric = method_call.to_metric expect(metric).to be_an_instance_of(Gitlab::Metrics::Metric) - expect(metric.series).to eq('foo') + expect(metric.series).to eq('rails_method_calls') expect(metric.values[:duration]).to be_a_kind_of(Numeric) expect(metric.values[:cpu_duration]).to be_a_kind_of(Numeric) diff --git a/spec/lib/gitlab/metrics/rack_middleware_spec.rb b/spec/lib/gitlab/metrics/rack_middleware_spec.rb index ec415f2bd85..b84387204ee 100644 --- a/spec/lib/gitlab/metrics/rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/rack_middleware_spec.rb @@ -18,34 +18,6 @@ describe Gitlab::Metrics::RackMiddleware do expect(middleware.call(env)).to eq('yay') end - it 'tags a transaction with the name and action of a controller' do - klass = double(:klass, name: 'TestController', content_type: 'text/html') - controller = double(:controller, class: klass, action_name: 'show') - - env['action_controller.instance'] = controller - - allow(app).to receive(:call).with(env) - - expect(middleware).to receive(:tag_controller) - .with(an_instance_of(Gitlab::Metrics::Transaction), env) - - middleware.call(env) - end - - it 'tags a transaction with the method and path of the route in the grape endpoint' do - route = double(:route, request_method: "GET", path: "/:version/projects/:id/archive(.:format)") - endpoint = double(:endpoint, route: route) - - env['api.endpoint'] = endpoint - - allow(app).to receive(:call).with(env) - - expect(middleware).to receive(:tag_endpoint) - .with(an_instance_of(Gitlab::Metrics::Transaction), env) - - middleware.call(env) - end - it 'tracks any raised exceptions' do expect(app).to receive(:call).with(env).and_raise(RuntimeError) @@ -60,7 +32,7 @@ describe Gitlab::Metrics::RackMiddleware do let(:transaction) { middleware.transaction_from_env(env) } it 'returns a Transaction' do - expect(transaction).to be_an_instance_of(Gitlab::Metrics::Transaction) + expect(transaction).to be_an_instance_of(Gitlab::Metrics::WebTransaction) end it 'stores the request method and URI in the transaction as values' do @@ -84,58 +56,4 @@ describe Gitlab::Metrics::RackMiddleware do end end end - - describe '#tag_controller' do - let(:transaction) { middleware.transaction_from_env(env) } - let(:content_type) { 'text/html' } - - before do - klass = double(:klass, name: 'TestController') - controller = double(:controller, class: klass, action_name: 'show', content_type: content_type) - - env['action_controller.instance'] = controller - end - - it 'tags a transaction with the name and action of a controller' do - middleware.tag_controller(transaction, env) - - expect(transaction.action).to eq('TestController#show') - end - - context 'when the response content type is not :html' do - let(:content_type) { 'application/json' } - - it 'appends the mime type to the transaction action' do - middleware.tag_controller(transaction, env) - - expect(transaction.action).to eq('TestController#show.json') - end - end - end - - describe '#tag_endpoint' do - let(:transaction) { middleware.transaction_from_env(env) } - - it 'tags a transaction with the method and path of the route in the grape endpount' do - route = double(:route, request_method: "GET", path: "/:version/projects/:id/archive(.:format)") - endpoint = double(:endpoint, route: route) - - env['api.endpoint'] = endpoint - - middleware.tag_endpoint(transaction, env) - - expect(transaction.action).to eq('Grape#GET /projects/:id/archive') - end - - it 'does not tag a transaction if route infos are missing' do - endpoint = double(:endpoint) - allow(endpoint).to receive(:route).and_raise - - env['api.endpoint'] = endpoint - - middleware.tag_endpoint(transaction, env) - - expect(transaction.action).to be_nil - end - end end diff --git a/spec/lib/gitlab/metrics/influx_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb index 999a9536d82..667e4747897 100644 --- a/spec/lib/gitlab/metrics/influx_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Metrics::InfluxSampler do +describe Gitlab::Metrics::Samplers::InfluxSampler do let(:sampler) { described_class.new(5) } after do diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb new file mode 100644 index 00000000000..53699327da1 --- /dev/null +++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb @@ -0,0 +1,90 @@ +require 'spec_helper' + +describe Gitlab::Metrics::Samplers::RubySampler do + let(:sampler) { described_class.new(5) } + + after do + Allocations.stop if Gitlab::Metrics.mri? + end + + describe '#sample' do + it 'samples various statistics' do + expect(Gitlab::Metrics::System).to receive(:memory_usage) + expect(Gitlab::Metrics::System).to receive(:file_descriptor_count) + expect(sampler).to receive(:sample_objects) + expect(sampler).to receive(:sample_gc) + + sampler.sample + end + + it 'adds a metric containing the memory usage' do + expect(Gitlab::Metrics::System).to receive(:memory_usage) + .and_return(9000) + + expect(sampler.metrics[:memory_usage]).to receive(:set) + .with({}, 9000) + .and_call_original + + sampler.sample + end + + it 'adds a metric containing the amount of open file descriptors' do + expect(Gitlab::Metrics::System).to receive(:file_descriptor_count) + .and_return(4) + + expect(sampler.metrics[:file_descriptors]).to receive(:set) + .with({}, 4) + .and_call_original + + sampler.sample + end + + it 'clears any GC profiles' do + expect(GC::Profiler).to receive(:clear) + + sampler.sample + end + end + + describe '#sample_gc' do + it 'adds a metric containing garbage collection time statistics' do + expect(GC::Profiler).to receive(:total_time).and_return(0.24) + + expect(sampler.metrics[:total_time]).to receive(:set) + .with({}, 240) + .and_call_original + + sampler.sample + end + + it 'adds a metric containing garbage collection statistics' do + GC.stat.keys.each do |key| + expect(sampler.metrics[key]).to receive(:set).with({}, anything).and_call_original + end + + sampler.sample + end + end + + if Gitlab::Metrics.mri? + describe '#sample_objects' do + it 'adds a metric containing the amount of allocated objects' do + expect(sampler.metrics[:objects_total]).to receive(:set) + .with(include(class: anything), be > 0) + .at_least(:once) + .and_call_original + + sampler.sample + end + + it 'ignores classes without a name' do + expect(Allocations).to receive(:to_hash).and_return({ Class.new => 4 }) + + expect(sampler.metrics[:objects_total]).not_to receive(:set) + .with(include(class: 'object_counts'), anything) + + sampler.sample + end + end + end +end diff --git a/spec/lib/gitlab/metrics/unicorn_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb index dc0d1f2e940..771b633a2b9 100644 --- a/spec/lib/gitlab/metrics/unicorn_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Metrics::UnicornSampler do +describe Gitlab::Metrics::Samplers::UnicornSampler do subject { described_class.new(1.second) } describe '#sample' do diff --git a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb index 0803ce42fac..6d69b5305d2 100644 --- a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb @@ -5,8 +5,8 @@ describe Gitlab::Metrics::SidekiqMiddleware do let(:message) { { 'args' => ['test'], 'enqueued_at' => Time.new(2016, 6, 23, 6, 59).to_f } } def run(worker, message) - expect(Gitlab::Metrics::Transaction).to receive(:new) - .with('TestWorker#perform') + expect(Gitlab::Metrics::BackgroundTransaction).to receive(:new) + .with(worker.class) .and_call_original expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:set) @@ -18,21 +18,18 @@ describe Gitlab::Metrics::SidekiqMiddleware do end describe '#call' do - it 'tracks the transaction' do - worker = double(:worker, class: double(:class, name: 'TestWorker')) + let(:test_worker_class) { double(:class, name: 'TestWorker') } + let(:worker) { double(:worker, class: test_worker_class) } + it 'tracks the transaction' do run(worker, message) end it 'tracks the transaction (for messages without `enqueued_at`)' do - worker = double(:worker, class: double(:class, name: 'TestWorker')) - run(worker, {}) end it 'tracks any raised exceptions' do - worker = double(:worker, class: double(:class, name: 'TestWorker')) - expect_any_instance_of(Gitlab::Metrics::Transaction) .to receive(:run).and_raise(RuntimeError) @@ -45,18 +42,5 @@ describe Gitlab::Metrics::SidekiqMiddleware do expect { middleware.call(worker, message, :test) } .to raise_error(RuntimeError) end - - it 'tags the metrics accordingly' do - tags = { one: 1, two: 2 } - worker = double(:worker, class: double(:class, name: 'TestWorker')) - allow(worker).to receive(:metrics_tags).and_return(tags) - - tags.each do |tag, value| - expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:add_tag) - .with(tag, value) - end - - run(worker, message) - end end end diff --git a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb index e7b595405a8..eca75a4fac1 100644 --- a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' describe Gitlab::Metrics::Subscribers::ActionView do - let(:transaction) { Gitlab::Metrics::Transaction.new } + let(:env) { {} } + let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) } let(:subscriber) { described_class.new } @@ -29,5 +30,13 @@ describe Gitlab::Metrics::Subscribers::ActionView do subscriber.render_template(event) end + + it 'observes view rendering time' do + expect(subscriber.send(:metric_view_rendering_duration_seconds)) + .to receive(:observe) + .with({ view: 'app/views/x.html.haml' }, 2.1) + + subscriber.render_template(event) + end end end diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb index ce6587e993f..9b3698fb4a8 100644 --- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb @@ -1,11 +1,12 @@ require 'spec_helper' describe Gitlab::Metrics::Subscribers::ActiveRecord do - let(:transaction) { Gitlab::Metrics::Transaction.new } + let(:env) { {} } + let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) } let(:subscriber) { described_class.new } let(:event) do - double(:event, duration: 0.2, + double(:event, duration: 2, payload: { sql: 'SELECT * FROM users WHERE id = 10' }) end @@ -20,16 +21,24 @@ describe Gitlab::Metrics::Subscribers::ActiveRecord do end describe 'with a current transaction' do + it 'observes sql_duration metric' do + expect(subscriber).to receive(:current_transaction) + .at_least(:once) + .and_return(transaction) + expect(subscriber.send(:metric_sql_duration_seconds)).to receive(:observe).with({}, 0.002) + subscriber.sql(event) + end + it 'increments the :sql_duration value' do expect(subscriber).to receive(:current_transaction) .at_least(:once) .and_return(transaction) expect(transaction).to receive(:increment) - .with(:sql_duration, 0.2) + .with(:sql_duration, 2, false) expect(transaction).to receive(:increment) - .with(:sql_count, 1) + .with(:sql_count, 1, false) subscriber.sql(event) end diff --git a/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb b/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb index f04dc8dcc02..58e28592cf9 100644 --- a/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb @@ -1,15 +1,16 @@ require 'spec_helper' describe Gitlab::Metrics::Subscribers::RailsCache do - let(:transaction) { Gitlab::Metrics::Transaction.new } + let(:env) { {} } + let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) } let(:subscriber) { described_class.new } let(:event) { double(:event, duration: 15.2) } describe '#cache_read' do it 'increments the cache_read duration' do - expect(subscriber).to receive(:increment) - .with(:cache_read, event.duration) + expect(subscriber).to receive(:observe) + .with(:read, event.duration) subscriber.cache_read(event) end @@ -17,7 +18,7 @@ describe Gitlab::Metrics::Subscribers::RailsCache do context 'with a transaction' do before do allow(subscriber).to receive(:current_transaction) - .and_return(transaction) + .and_return(transaction) end context 'with hit event' do @@ -25,9 +26,9 @@ describe Gitlab::Metrics::Subscribers::RailsCache do it 'increments the cache_read_hit count' do expect(transaction).to receive(:increment) - .with(:cache_read_hit_count, 1) + .with(:cache_read_hit_count, 1, false) expect(transaction).to receive(:increment) - .with(any_args).at_least(1) # Other calls + .with(any_args).at_least(1) # Other calls subscriber.cache_read(event) end @@ -37,7 +38,7 @@ describe Gitlab::Metrics::Subscribers::RailsCache do it 'does not increment cache read miss' do expect(transaction).not_to receive(:increment) - .with(:cache_read_hit_count, 1) + .with(:cache_read_hit_count, 1) subscriber.cache_read(event) end @@ -49,9 +50,15 @@ describe Gitlab::Metrics::Subscribers::RailsCache do it 'increments the cache_read_miss count' do expect(transaction).to receive(:increment) - .with(:cache_read_miss_count, 1) + .with(:cache_read_miss_count, 1, false) expect(transaction).to receive(:increment) - .with(any_args).at_least(1) # Other calls + .with(any_args).at_least(1) # Other calls + + subscriber.cache_read(event) + end + + it 'increments the cache_read_miss total' do + expect(subscriber.send(:metric_cache_misses_total)).to receive(:increment).with({}) subscriber.cache_read(event) end @@ -61,7 +68,13 @@ describe Gitlab::Metrics::Subscribers::RailsCache do it 'does not increment cache read miss' do expect(transaction).not_to receive(:increment) - .with(:cache_read_miss_count, 1) + .with(:cache_read_miss_count, 1) + + subscriber.cache_read(event) + end + + it 'does not increment cache_read_miss total' do + expect(subscriber.send(:metric_cache_misses_total)).not_to receive(:increment).with({}) subscriber.cache_read(event) end @@ -71,27 +84,27 @@ describe Gitlab::Metrics::Subscribers::RailsCache do end describe '#cache_write' do - it 'increments the cache_write duration' do - expect(subscriber).to receive(:increment) - .with(:cache_write, event.duration) + it 'observes write duration' do + expect(subscriber).to receive(:observe) + .with(:write, event.duration) subscriber.cache_write(event) end end describe '#cache_delete' do - it 'increments the cache_delete duration' do - expect(subscriber).to receive(:increment) - .with(:cache_delete, event.duration) + it 'observes delete duration' do + expect(subscriber).to receive(:observe) + .with(:delete, event.duration) subscriber.cache_delete(event) end end describe '#cache_exist?' do - it 'increments the cache_exists duration' do - expect(subscriber).to receive(:increment) - .with(:cache_exists, event.duration) + it 'observes the exists duration' do + expect(subscriber).to receive(:observe) + .with(:exists, event.duration) subscriber.cache_exist?(event) end @@ -109,12 +122,12 @@ describe Gitlab::Metrics::Subscribers::RailsCache do context 'with a transaction' do before do allow(subscriber).to receive(:current_transaction) - .and_return(transaction) + .and_return(transaction) end it 'increments the cache_read_hit count' do expect(transaction).to receive(:increment) - .with(:cache_read_hit_count, 1) + .with(:cache_read_hit_count, 1) subscriber.cache_fetch_hit(event) end @@ -133,47 +146,61 @@ describe Gitlab::Metrics::Subscribers::RailsCache do context 'with a transaction' do before do allow(subscriber).to receive(:current_transaction) - .and_return(transaction) + .and_return(transaction) end it 'increments the cache_fetch_miss count' do expect(transaction).to receive(:increment) - .with(:cache_read_miss_count, 1) + .with(:cache_read_miss_count, 1) + + subscriber.cache_generate(event) + end + + it 'increments the cache_read_miss total' do + expect(subscriber.send(:metric_cache_misses_total)).to receive(:increment).with({}) subscriber.cache_generate(event) end end end - describe '#increment' do + describe '#observe' do context 'without a transaction' do it 'returns' do expect(transaction).not_to receive(:increment) - subscriber.increment(:foo, 15.2) + subscriber.observe(:foo, 15.2) end end context 'with a transaction' do before do allow(subscriber).to receive(:current_transaction) - .and_return(transaction) + .and_return(transaction) end it 'increments the total and specific cache duration' do expect(transaction).to receive(:increment) - .with(:cache_duration, event.duration) + .with(:cache_duration, event.duration, false) expect(transaction).to receive(:increment) - .with(:cache_count, 1) + .with(:cache_count, 1, false) expect(transaction).to receive(:increment) - .with(:cache_delete_duration, event.duration) + .with(:cache_delete_duration, event.duration, false) expect(transaction).to receive(:increment) - .with(:cache_delete_count, 1) + .with(:cache_delete_count, 1, false) + + subscriber.observe(:delete, event.duration) + end + + it 'observes cache metric' do + expect(subscriber.send(:metric_cache_operation_duration_seconds)) + .to receive(:observe) + .with(transaction.labels.merge(operation: :delete), event.duration / 1000.0) - subscriber.increment(:cache_delete, event.duration) + subscriber.observe(:delete, event.duration) end end end diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/web_transaction_spec.rb index 3779af81512..1d162f53a13 100644 --- a/spec/lib/gitlab/metrics/transaction_spec.rb +++ b/spec/lib/gitlab/metrics/web_transaction_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' -describe Gitlab::Metrics::Transaction do - let(:transaction) { described_class.new } +describe Gitlab::Metrics::WebTransaction do + let(:env) { {} } + let(:transaction) { described_class.new(env) } describe '#duration' do it 'returns the duration of a transaction in seconds' do @@ -48,7 +49,7 @@ describe Gitlab::Metrics::Transaction do describe '#method_call_for' do it 'returns a MethodCall' do - method = transaction.method_call_for('Foo#bar') + method = transaction.method_call_for('Foo#bar', :Foo, '#bar') expect(method).to be_an_instance_of(Gitlab::Metrics::MethodCall) end @@ -85,14 +86,6 @@ describe Gitlab::Metrics::Transaction do end end - describe '#add_tag' do - it 'adds a tag' do - transaction.add_tag(:foo, 'bar') - - expect(transaction.tags).to eq({ foo: 'bar' }) - end - end - describe '#finish' do it 'tracks the transaction details and submits them to Sidekiq' do expect(transaction).to receive(:track_self) @@ -127,7 +120,7 @@ describe Gitlab::Metrics::Transaction do end it 'adds the action as a tag for every metric' do - transaction.action = 'Foo#bar' + allow(transaction).to receive(:labels).and_return(controller: 'Foo', action: 'bar') transaction.track_self hash = { @@ -144,7 +137,8 @@ describe Gitlab::Metrics::Transaction do end it 'does not add an action tag for events' do - transaction.action = 'Foo#bar' + allow(transaction).to receive(:labels).and_return(controller: 'Foo', action: 'bar') + transaction.add_event(:meow) hash = { @@ -161,6 +155,61 @@ describe Gitlab::Metrics::Transaction do end end + describe '#labels' do + context 'when request goes to Grape endpoint' do + before do + route = double(:route, request_method: 'GET', path: '/:version/projects/:id/archive(.:format)') + endpoint = double(:endpoint, route: route) + + env['api.endpoint'] = endpoint + end + it 'provides labels with the method and path of the route in the grape endpoint' do + expect(transaction.labels).to eq({ controller: 'Grape', action: 'GET /projects/:id/archive' }) + expect(transaction.action).to eq('Grape#GET /projects/:id/archive') + end + + it 'does not provide labels if route infos are missing' do + endpoint = double(:endpoint) + allow(endpoint).to receive(:route).and_raise + + env['api.endpoint'] = endpoint + + expect(transaction.labels).to eq({}) + expect(transaction.action).to be_nil + end + end + + context 'when request goes to ActionController' do + let(:content_type) { 'text/html' } + + before do + klass = double(:klass, name: 'TestController') + controller = double(:controller, class: klass, action_name: 'show', content_type: content_type) + + env['action_controller.instance'] = controller + end + + it 'tags a transaction with the name and action of a controller' do + expect(transaction.labels).to eq({ controller: 'TestController', action: 'show' }) + expect(transaction.action).to eq('TestController#show') + end + + context 'when the response content type is not :html' do + let(:content_type) { 'application/json' } + + it 'appends the mime type to the transaction action' do + expect(transaction.labels).to eq({ controller: 'TestController', action: 'show.json' }) + expect(transaction.action).to eq('TestController#show.json') + end + end + end + + it 'returns no labels when no route information is present in env' do + expect(transaction.labels).to eq({}) + expect(transaction.action).to eq(nil) + end + end + describe '#add_event' do it 'adds a metric' do transaction.add_event(:meow) diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb index 599b8807d8d..1619fbd88b1 100644 --- a/spec/lib/gitlab/metrics_spec.rb +++ b/spec/lib/gitlab/metrics_spec.rb @@ -115,7 +115,7 @@ describe Gitlab::Metrics do end context 'with a transaction' do - let(:transaction) { Gitlab::Metrics::Transaction.new } + let(:transaction) { Gitlab::Metrics::WebTransaction.new({}) } before do allow(described_class).to receive(:current_transaction) @@ -124,13 +124,13 @@ describe Gitlab::Metrics do it 'adds a metric to the current transaction' do expect(transaction).to receive(:increment) - .with('foo_real_time', a_kind_of(Numeric)) + .with('foo_real_time', a_kind_of(Numeric), false) expect(transaction).to receive(:increment) - .with('foo_cpu_time', a_kind_of(Numeric)) + .with('foo_cpu_time', a_kind_of(Numeric), false) expect(transaction).to receive(:increment) - .with('foo_call_count', 1) + .with('foo_call_count', 1, false) described_class.measure(:foo) { 10 } end @@ -143,31 +143,6 @@ describe Gitlab::Metrics do end end - describe '.tag_transaction' do - context 'without a transaction' do - it 'does nothing' do - expect_any_instance_of(Gitlab::Metrics::Transaction) - .not_to receive(:add_tag) - - described_class.tag_transaction(:foo, 'bar') - end - end - - context 'with a transaction' do - let(:transaction) { Gitlab::Metrics::Transaction.new } - - it 'adds the tag to the transaction' do - expect(described_class).to receive(:current_transaction) - .and_return(transaction) - - expect(transaction).to receive(:add_tag) - .with(:foo, 'bar') - - described_class.tag_transaction(:foo, 'bar') - end - end - end - describe '.action=' do context 'without a transaction' do it 'does nothing' do @@ -180,7 +155,7 @@ describe Gitlab::Metrics do context 'with a transaction' do it 'sets the action of a transaction' do - trans = Gitlab::Metrics::Transaction.new + trans = Gitlab::Metrics::WebTransaction.new({}) expect(described_class).to receive(:current_transaction) .and_return(trans) @@ -210,7 +185,7 @@ describe Gitlab::Metrics do context 'with a transaction' do it 'adds an event' do - transaction = Gitlab::Metrics::Transaction.new + transaction = Gitlab::Metrics::WebTransaction.new({}) expect(transaction).to receive(:add_event).with(:meow) @@ -224,7 +199,7 @@ describe Gitlab::Metrics do shared_examples 'prometheus metrics API' do describe '#counter' do - subject { described_class.counter(:couter, 'doc') } + subject { described_class.counter(:counter, 'doc') } describe '#increment' do it 'successfully calls #increment without arguments' do @@ -280,7 +255,7 @@ describe Gitlab::Metrics do it_behaves_like 'prometheus metrics API' describe '#null_metric' do - subject { described_class.provide_metric(:test) } + subject { described_class.send(:provide_metric, :test) } it { is_expected.to be_a(Gitlab::Metrics::NullMetric) } end @@ -321,7 +296,7 @@ describe Gitlab::Metrics do it_behaves_like 'prometheus metrics API' describe '#null_metric' do - subject { described_class.provide_metric(:test) } + subject { described_class.send(:provide_metric, :test) } it { is_expected.to be_nil } end diff --git a/spec/lib/gitlab/middleware/rails_queue_duration_spec.rb b/spec/lib/gitlab/middleware/rails_queue_duration_spec.rb index 88107536c9e..14f2c3cb86f 100644 --- a/spec/lib/gitlab/middleware/rails_queue_duration_spec.rb +++ b/spec/lib/gitlab/middleware/rails_queue_duration_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::Middleware::RailsQueueDuration do let(:app) { double(:app) } let(:middleware) { described_class.new(app) } let(:env) { {} } - let(:transaction) { double(:transaction) } + let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) } before do expect(app).to receive(:call).with(env).and_return('yay') @@ -30,6 +30,16 @@ describe Gitlab::Middleware::RailsQueueDuration do expect(transaction).to receive(:set).with(:rails_queue_duration, an_instance_of(Float)) expect(middleware.call(env)).to eq('yay') end + + it 'observes rails queue duration metrics and calls the app when the header is present' do + env['HTTP_GITLAB_WORKHORSE_PROXY_START'] = '2000000000' + + expect(middleware.send(:metric_rails_queue_duration_seconds)).to receive(:observe).with(transaction.labels, 1) + + Timecop.freeze(Time.at(3)) do + expect(middleware.call(env)).to eq('yay') + end + end end end end diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb index f1f188cbfb5..ee63c9338c5 100644 --- a/spec/lib/gitlab/path_regex_spec.rb +++ b/spec/lib/gitlab/path_regex_spec.rb @@ -68,14 +68,27 @@ describe Gitlab::PathRegex do message end - let(:all_routes) do + let(:all_non_legacy_routes) do route_set = Rails.application.routes routes_collection = route_set.routes routes_array = routes_collection.routes - routes_array.map { |route| route.path.spec.to_s } + + non_legacy_routes = routes_array.reject do |route| + route.name.to_s =~ /legacy_(\w*)_redirect/ + end + + non_deprecated_redirect_routes = non_legacy_routes.reject do |route| + app = route.app + # `app.app` is either another app, or `self`. We want to find the final app. + app = app.app while app.try(:app) && app.app != app + + app.is_a?(ActionDispatch::Routing::PathRedirect) && app.block.include?('/-/') + end + + non_deprecated_redirect_routes.map { |route| route.path.spec.to_s } end - let(:routes_without_format) { all_routes.map { |path| without_format(path) } } + let(:routes_without_format) { all_non_legacy_routes.map { |path| without_format(path) } } # Routes not starting with `/:` or `/*` # all routes not starting with a param diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index a7b65e94706..a4c1113ae37 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -60,9 +60,9 @@ describe Gitlab::UsageData do deploy_keys deployments environments - gcp_clusters - gcp_clusters_enabled - gcp_clusters_disabled + clusters + clusters_enabled + clusters_disabled in_review_folder groups issues diff --git a/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb new file mode 100644 index 00000000000..9f41534441b --- /dev/null +++ b/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb @@ -0,0 +1,166 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20171013104327_migrate_gcp_clusters_to_new_clusters_architectures.rb') + +describe MigrateGcpClustersToNewClustersArchitectures, :migration do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:service) { create(:kubernetes_service, project: project) } + + context 'when cluster is being created' do + let(:project_id) { project.id } + let(:user_id) { user.id } + let(:service_id) { service.id } + let(:status) { 2 } # creating + let(:gcp_cluster_size) { 1 } + let(:created_at) { "'2017-10-17 20:24:02'" } + let(:updated_at) { "'2017-10-17 20:28:44'" } + let(:enabled) { true } + let(:status_reason) { "''" } + let(:project_namespace) { "'sample-app'" } + let(:endpoint) { 'NULL' } + let(:ca_cert) { 'NULL' } + let(:encrypted_kubernetes_token) { 'NULL' } + let(:encrypted_kubernetes_token_iv) { 'NULL' } + let(:username) { 'NULL' } + let(:encrypted_password) { 'NULL' } + let(:encrypted_password_iv) { 'NULL' } + let(:gcp_project_id) { "'gcp_project_id'" } + let(:gcp_cluster_zone) { "'gcp_cluster_zone'" } + let(:gcp_cluster_name) { "'gcp_cluster_name'" } + let(:gcp_machine_type) { "'gcp_machine_type'" } + let(:gcp_operation_id) { 'NULL' } + let(:encrypted_gcp_token) { "'encrypted_gcp_token'" } + let(:encrypted_gcp_token_iv) { "'encrypted_gcp_token_iv'" } + + let(:cluster) { Clusters::Cluster.last } + let(:cluster_id) { cluster.id } + + before do + ActiveRecord::Base.connection.execute <<-SQL + INSERT INTO gcp_clusters (project_id, user_id, service_id, status, gcp_cluster_size, created_at, updated_at, enabled, status_reason, project_namespace, endpoint, ca_cert, encrypted_kubernetes_token, encrypted_kubernetes_token_iv, username, encrypted_password, encrypted_password_iv, gcp_project_id, gcp_cluster_zone, gcp_cluster_name, gcp_machine_type, gcp_operation_id, encrypted_gcp_token, encrypted_gcp_token_iv) + VALUES (#{project_id}, #{user_id}, #{service_id}, #{status}, #{gcp_cluster_size}, #{created_at}, #{updated_at}, #{enabled}, #{status_reason}, #{project_namespace}, #{endpoint}, #{ca_cert}, #{encrypted_kubernetes_token}, #{encrypted_kubernetes_token_iv}, #{username}, #{encrypted_password}, #{encrypted_password_iv}, #{gcp_project_id}, #{gcp_cluster_zone}, #{gcp_cluster_name}, #{gcp_machine_type}, #{gcp_operation_id}, #{encrypted_gcp_token}, #{encrypted_gcp_token_iv}); + SQL + end + + it 'correctly migrate to new clusters architectures' do + migrate! + + expect(Clusters::Cluster.count).to eq(1) + expect(Clusters::Project.count).to eq(1) + expect(Clusters::Providers::Gcp.count).to eq(1) + expect(Clusters::Platforms::Kubernetes.count).to eq(1) + + expect(cluster.user).to eq(user) + expect(cluster.enabled).to be_truthy + expect(cluster.name).to eq(gcp_cluster_name.delete!("'")) + expect(cluster.provider_type).to eq('gcp') + expect(cluster.platform_type).to eq('kubernetes') + + expect(cluster.project).to eq(project) + expect(project.cluster).to eq(cluster) + + expect(cluster.provider_gcp.cluster).to eq(cluster) + expect(cluster.provider_gcp.status).to eq(status) + expect(cluster.provider_gcp.status_reason).to eq(tr(status_reason)) + expect(cluster.provider_gcp.gcp_project_id).to eq(tr(gcp_project_id)) + expect(cluster.provider_gcp.zone).to eq(tr(gcp_cluster_zone)) + expect(cluster.provider_gcp.num_nodes).to eq(gcp_cluster_size) + expect(cluster.provider_gcp.machine_type).to eq(tr(gcp_machine_type)) + expect(cluster.provider_gcp.operation_id).to be_nil + expect(cluster.provider_gcp.endpoint).to be_nil + expect(cluster.provider_gcp.encrypted_access_token).to eq(tr(encrypted_gcp_token)) + expect(cluster.provider_gcp.encrypted_access_token_iv).to eq(tr(encrypted_gcp_token_iv)) + + expect(cluster.platform_kubernetes.cluster).to eq(cluster) + expect(cluster.platform_kubernetes.api_url).to be_nil + expect(cluster.platform_kubernetes.ca_cert).to be_nil + expect(cluster.platform_kubernetes.namespace).to eq(tr(project_namespace)) + expect(cluster.platform_kubernetes.username).to be_nil + expect(cluster.platform_kubernetes.encrypted_password).to be_nil + expect(cluster.platform_kubernetes.encrypted_password_iv).to be_nil + expect(cluster.platform_kubernetes.encrypted_token).to be_nil + expect(cluster.platform_kubernetes.encrypted_token_iv).to be_nil + end + end + + context 'when cluster has been created' do + let(:project_id) { project.id } + let(:user_id) { user.id } + let(:service_id) { service.id } + let(:status) { 3 } # created + let(:gcp_cluster_size) { 1 } + let(:created_at) { "'2017-10-17 20:24:02'" } + let(:updated_at) { "'2017-10-17 20:28:44'" } + let(:enabled) { true } + let(:status_reason) { "'general error'" } + let(:project_namespace) { "'sample-app'" } + let(:endpoint) { "'111.111.111.111'" } + let(:ca_cert) { "'ca_cert'" } + let(:encrypted_kubernetes_token) { "'encrypted_kubernetes_token'" } + let(:encrypted_kubernetes_token_iv) { "'encrypted_kubernetes_token_iv'" } + let(:username) { "'username'" } + let(:encrypted_password) { "'encrypted_password'" } + let(:encrypted_password_iv) { "'encrypted_password_iv'" } + let(:gcp_project_id) { "'gcp_project_id'" } + let(:gcp_cluster_zone) { "'gcp_cluster_zone'" } + let(:gcp_cluster_name) { "'gcp_cluster_name'" } + let(:gcp_machine_type) { "'gcp_machine_type'" } + let(:gcp_operation_id) { "'gcp_operation_id'" } + let(:encrypted_gcp_token) { "'encrypted_gcp_token'" } + let(:encrypted_gcp_token_iv) { "'encrypted_gcp_token_iv'" } + + let(:cluster) { Clusters::Cluster.last } + let(:cluster_id) { cluster.id } + + before do + ActiveRecord::Base.connection.execute <<-SQL + INSERT INTO gcp_clusters (project_id, user_id, service_id, status, gcp_cluster_size, created_at, updated_at, enabled, status_reason, project_namespace, endpoint, ca_cert, encrypted_kubernetes_token, encrypted_kubernetes_token_iv, username, encrypted_password, encrypted_password_iv, gcp_project_id, gcp_cluster_zone, gcp_cluster_name, gcp_machine_type, gcp_operation_id, encrypted_gcp_token, encrypted_gcp_token_iv) + VALUES (#{project_id}, #{user_id}, #{service_id}, #{status}, #{gcp_cluster_size}, #{created_at}, #{updated_at}, #{enabled}, #{status_reason}, #{project_namespace}, #{endpoint}, #{ca_cert}, #{encrypted_kubernetes_token}, #{encrypted_kubernetes_token_iv}, #{username}, #{encrypted_password}, #{encrypted_password_iv}, #{gcp_project_id}, #{gcp_cluster_zone}, #{gcp_cluster_name}, #{gcp_machine_type}, #{gcp_operation_id}, #{encrypted_gcp_token}, #{encrypted_gcp_token_iv}); + SQL + end + + it 'correctly migrate to new clusters architectures' do + migrate! + + expect(Clusters::Cluster.count).to eq(1) + expect(Clusters::Project.count).to eq(1) + expect(Clusters::Providers::Gcp.count).to eq(1) + expect(Clusters::Platforms::Kubernetes.count).to eq(1) + + expect(cluster.user).to eq(user) + expect(cluster.enabled).to be_truthy + expect(cluster.name).to eq(tr(gcp_cluster_name)) + expect(cluster.provider_type).to eq('gcp') + expect(cluster.platform_type).to eq('kubernetes') + + expect(cluster.project).to eq(project) + expect(project.cluster).to eq(cluster) + + expect(cluster.provider_gcp.cluster).to eq(cluster) + expect(cluster.provider_gcp.status).to eq(status) + expect(cluster.provider_gcp.status_reason).to eq(tr(status_reason)) + expect(cluster.provider_gcp.gcp_project_id).to eq(tr(gcp_project_id)) + expect(cluster.provider_gcp.zone).to eq(tr(gcp_cluster_zone)) + expect(cluster.provider_gcp.num_nodes).to eq(gcp_cluster_size) + expect(cluster.provider_gcp.machine_type).to eq(tr(gcp_machine_type)) + expect(cluster.provider_gcp.operation_id).to eq(tr(gcp_operation_id)) + expect(cluster.provider_gcp.endpoint).to eq(tr(endpoint)) + expect(cluster.provider_gcp.encrypted_access_token).to eq(tr(encrypted_gcp_token)) + expect(cluster.provider_gcp.encrypted_access_token_iv).to eq(tr(encrypted_gcp_token_iv)) + + expect(cluster.platform_kubernetes.cluster).to eq(cluster) + expect(cluster.platform_kubernetes.api_url).to eq('https://' + tr(endpoint)) + expect(cluster.platform_kubernetes.ca_cert).to eq(tr(ca_cert)) + expect(cluster.platform_kubernetes.namespace).to eq(tr(project_namespace)) + expect(cluster.platform_kubernetes.username).to eq(tr(username)) + expect(cluster.platform_kubernetes.encrypted_password).to eq(tr(encrypted_password)) + expect(cluster.platform_kubernetes.encrypted_password_iv).to eq(tr(encrypted_password_iv)) + expect(cluster.platform_kubernetes.encrypted_token).to eq(tr(encrypted_kubernetes_token)) + expect(cluster.platform_kubernetes.encrypted_token_iv).to eq(tr(encrypted_kubernetes_token_iv)) + end + end + + def tr(s) + s.delete("'") + end +end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb new file mode 100644 index 00000000000..12123a3d753 --- /dev/null +++ b/spec/models/clusters/cluster_spec.rb @@ -0,0 +1,181 @@ +require 'spec_helper' + +describe Clusters::Cluster do + it { is_expected.to belong_to(:user) } + it { is_expected.to have_many(:projects) } + it { is_expected.to have_one(:provider_gcp) } + it { is_expected.to have_one(:platform_kubernetes) } + it { is_expected.to delegate_method(:status).to(:provider) } + it { is_expected.to delegate_method(:status_reason).to(:provider) } + it { is_expected.to delegate_method(:status_name).to(:provider) } + it { is_expected.to delegate_method(:on_creation?).to(:provider) } + it { is_expected.to delegate_method(:update_kubernetes_integration!).to(:platform) } + it { is_expected.to respond_to :project } + + describe '.enabled' do + subject { described_class.enabled } + + let!(:cluster) { create(:cluster, enabled: true) } + + before do + create(:cluster, enabled: false) + end + + it { is_expected.to contain_exactly(cluster) } + end + + describe '.disabled' do + subject { described_class.disabled } + + let!(:cluster) { create(:cluster, enabled: false) } + + before do + create(:cluster, enabled: true) + end + + it { is_expected.to contain_exactly(cluster) } + end + + describe 'validation' do + subject { cluster.valid? } + + context 'when validates name' do + context 'when provided by user' do + let!(:cluster) { build(:cluster, :provided_by_user, name: name) } + + context 'when name is empty' do + let(:name) { '' } + + it { is_expected.to be_falsey } + end + + context 'when name is nil' do + let(:name) { nil } + + it { is_expected.to be_falsey } + end + + context 'when name is present' do + let(:name) { 'cluster-name-1' } + + it { is_expected.to be_truthy } + end + end + + context 'when provided by gcp' do + let!(:cluster) { build(:cluster, :provided_by_gcp, name: name) } + + context 'when name is shorter than 1' do + let(:name) { '' } + + it { is_expected.to be_falsey } + end + + context 'when name is longer than 63' do + let(:name) { 'a' * 64 } + + it { is_expected.to be_falsey } + end + + context 'when name includes invalid character' do + let(:name) { '!!!!!!' } + + it { is_expected.to be_falsey } + end + + context 'when name is present' do + let(:name) { 'cluster-name-1' } + + it { is_expected.to be_truthy } + end + + context 'when record is persisted' do + let(:name) { 'cluster-name-1' } + + before do + cluster.save! + end + + context 'when name is changed' do + before do + cluster.name = 'new-cluster-name' + end + + it { is_expected.to be_falsey } + end + + context 'when name is same' do + before do + cluster.name = name + end + + it { is_expected.to be_truthy } + end + end + end + end + + context 'when validates restrict_modification' do + context 'when creation is on going' do + let!(:cluster) { create(:cluster, :providing_by_gcp) } + + it { expect(cluster.update(enabled: false)).to be_falsey } + end + + context 'when creation is done' do + let!(:cluster) { create(:cluster, :provided_by_gcp) } + + it { expect(cluster.update(enabled: false)).to be_truthy } + end + end + end + + describe '#provider' do + subject { cluster.provider } + + context 'when provider is gcp' do + let(:cluster) { create(:cluster, :provided_by_gcp) } + + it 'returns a provider' do + is_expected.to eq(cluster.provider_gcp) + expect(subject.class.name.deconstantize).to eq(Clusters::Providers.to_s) + end + end + + context 'when provider is user' do + let(:cluster) { create(:cluster, :provided_by_user) } + + it { is_expected.to be_nil } + end + end + + describe '#platform' do + subject { cluster.platform } + + context 'when platform is kubernetes' do + let(:cluster) { create(:cluster, :provided_by_user) } + + it 'returns a platform' do + is_expected.to eq(cluster.platform_kubernetes) + expect(subject.class.name.deconstantize).to eq(Clusters::Platforms.to_s) + end + end + end + + describe '#first_project' do + subject { cluster.first_project } + + context 'when cluster belongs to a project' do + let(:cluster) { create(:cluster, :project) } + let(:project) { Clusters::Project.find_by_cluster_id(cluster.id).project } + + it { is_expected.to eq(project) } + end + + context 'when cluster does not belong to projects' do + let(:cluster) { create(:cluster) } + + it { is_expected.to be_nil } + end + end +end diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb new file mode 100644 index 00000000000..ed76be703a5 --- /dev/null +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -0,0 +1,188 @@ +require 'spec_helper' + +describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching do + include KubernetesHelpers + include ReactiveCachingHelpers + + it { is_expected.to belong_to(:cluster) } + it { is_expected.to respond_to :ca_pem } + + describe 'before_validation' do + context 'when namespace includes upper case' do + let(:kubernetes) { create(:cluster_platform_kubernetes, :configured, namespace: namespace) } + let(:namespace) { 'ABC' } + + it 'converts to lower case' do + expect(kubernetes.namespace).to eq('abc') + end + end + end + + describe 'validation' do + subject { kubernetes.valid? } + + context 'when validates namespace' do + let(:kubernetes) { build(:cluster_platform_kubernetes, :configured, namespace: namespace) } + + context 'when namespace is blank' do + let(:namespace) { '' } + + it { is_expected.to be_truthy } + end + + context 'when namespace is longer than 63' do + let(:namespace) { 'a' * 64 } + + it { is_expected.to be_falsey } + end + + context 'when namespace includes invalid character' do + let(:namespace) { '!!!!!!' } + + it { is_expected.to be_falsey } + end + + context 'when namespace is vaild' do + let(:namespace) { 'namespace-123' } + + it { is_expected.to be_truthy } + end + end + + context 'when validates api_url' do + let(:kubernetes) { build(:cluster_platform_kubernetes, :configured) } + + before do + kubernetes.api_url = api_url + end + + context 'when api_url is invalid url' do + let(:api_url) { '!!!!!!' } + + it { expect(kubernetes.save).to be_falsey } + end + + context 'when api_url is nil' do + let(:api_url) { nil } + + it { expect(kubernetes.save).to be_falsey } + end + + context 'when api_url is valid url' do + let(:api_url) { 'https://111.111.111.111' } + + it { expect(kubernetes.save).to be_truthy } + end + end + + context 'when validates token' do + let(:kubernetes) { build(:cluster_platform_kubernetes, :configured) } + + before do + kubernetes.token = token + end + + context 'when token is nil' do + let(:token) { nil } + + it { expect(kubernetes.save).to be_falsey } + end + end + end + + describe 'after_save from Clusters::Cluster' do + context 'when platform_kubernetes is being cerated' do + let(:enabled) { true } + let(:project) { create(:project) } + let(:cluster) { build(:cluster, provider_type: :gcp, platform_type: :kubernetes, platform_kubernetes: platform, provider_gcp: provider, enabled: enabled, projects: [project]) } + let(:platform) { build(:cluster_platform_kubernetes, :configured) } + let(:provider) { build(:cluster_provider_gcp) } + let(:kubernetes_service) { project.kubernetes_service } + + it 'updates KubernetesService' do + cluster.save! + + expect(kubernetes_service.active).to eq(enabled) + expect(kubernetes_service.api_url).to eq(platform.api_url) + expect(kubernetes_service.namespace).to eq(platform.namespace) + expect(kubernetes_service.ca_pem).to eq(platform.ca_cert) + end + end + + context 'when platform_kubernetes has been created' do + let(:enabled) { false } + let!(:project) { create(:project) } + let!(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } + let(:platform) { cluster.platform } + let(:kubernetes_service) { project.kubernetes_service } + + it 'updates KubernetesService' do + cluster.update(enabled: enabled) + + expect(kubernetes_service.active).to eq(enabled) + end + end + + context 'when kubernetes_service has been configured without cluster integration' do + let!(:project) { create(:project) } + let(:cluster) { build(:cluster, provider_type: :gcp, platform_type: :kubernetes, platform_kubernetes: platform, provider_gcp: provider, projects: [project]) } + let(:platform) { build(:cluster_platform_kubernetes, :configured, api_url: 'https://111.111.111.111') } + let(:provider) { build(:cluster_provider_gcp) } + + before do + create(:kubernetes_service, project: project) + end + + it 'raises an error' do + expect { cluster.save! }.to raise_error('Kubernetes service already configured') + end + end + end + + describe '#actual_namespace' do + subject { kubernetes.actual_namespace } + + let!(:cluster) { create(:cluster, :project, platform_kubernetes: kubernetes) } + let(:project) { cluster.project } + let(:kubernetes) { create(:cluster_platform_kubernetes, :configured, namespace: namespace) } + + context 'when namespace is present' do + let(:namespace) { 'namespace-123' } + + it { is_expected.to eq(namespace) } + end + + context 'when namespace is not present' do + let(:namespace) { nil } + + it { is_expected.to eq("#{project.path}-#{project.id}") } + end + end + + describe '.namespace_for_project' do + subject { described_class.namespace_for_project(project) } + + let(:project) { create(:project) } + + it { is_expected.to eq("#{project.path}-#{project.id}") } + end + + describe '#default_namespace' do + subject { kubernetes.default_namespace } + + let(:kubernetes) { create(:cluster_platform_kubernetes, :configured) } + + context 'when cluster belongs to a project' do + let!(:cluster) { create(:cluster, :project, platform_kubernetes: kubernetes) } + let(:project) { cluster.project } + + it { is_expected.to eq("#{project.path}-#{project.id}") } + end + + context 'when cluster belongs to nothing' do + let!(:cluster) { create(:cluster, platform_kubernetes: kubernetes) } + + it { is_expected.to be_nil } + end + end +end diff --git a/spec/models/clusters/project_spec.rb b/spec/models/clusters/project_spec.rb new file mode 100644 index 00000000000..7d75d6ab345 --- /dev/null +++ b/spec/models/clusters/project_spec.rb @@ -0,0 +1,6 @@ +require 'spec_helper' + +describe Clusters::Project do + it { is_expected.to belong_to(:cluster) } + it { is_expected.to belong_to(:project) } +end diff --git a/spec/models/clusters/providers/gcp_spec.rb b/spec/models/clusters/providers/gcp_spec.rb new file mode 100644 index 00000000000..ecd0a08c953 --- /dev/null +++ b/spec/models/clusters/providers/gcp_spec.rb @@ -0,0 +1,183 @@ +require 'spec_helper' + +describe Clusters::Providers::Gcp do + it { is_expected.to belong_to(:cluster) } + it { is_expected.to validate_presence_of(:zone) } + + describe 'default_value_for' do + let(:gcp) { build(:cluster_provider_gcp) } + + it "has default value" do + expect(gcp.zone).to eq('us-central1-a') + expect(gcp.num_nodes).to eq(3) + expect(gcp.machine_type).to eq('n1-standard-4') + end + end + + describe 'validation' do + subject { gcp.valid? } + + context 'when validates gcp_project_id' do + let(:gcp) { build(:cluster_provider_gcp, gcp_project_id: gcp_project_id) } + + context 'when gcp_project_id is shorter than 1' do + let(:gcp_project_id) { '' } + + it { is_expected.to be_falsey } + end + + context 'when gcp_project_id is longer than 63' do + let(:gcp_project_id) { 'a' * 64 } + + it { is_expected.to be_falsey } + end + + context 'when gcp_project_id includes invalid character' do + let(:gcp_project_id) { '!!!!!!' } + + it { is_expected.to be_falsey } + end + + context 'when gcp_project_id is valid' do + let(:gcp_project_id) { 'gcp-project-1' } + + it { is_expected.to be_truthy } + end + end + + context 'when validates num_nodes' do + let(:gcp) { build(:cluster_provider_gcp, num_nodes: num_nodes) } + + context 'when num_nodes is string' do + let(:num_nodes) { 'A3' } + + it { is_expected.to be_falsey } + end + + context 'when num_nodes is nil' do + let(:num_nodes) { nil } + + it { is_expected.to be_falsey } + end + + context 'when num_nodes is smaller than 1' do + let(:num_nodes) { 0 } + + it { is_expected.to be_falsey } + end + + context 'when num_nodes is valid' do + let(:num_nodes) { 3 } + + it { is_expected.to be_truthy } + end + end + end + + describe '#state_machine' do + context 'when any => [:created]' do + let(:gcp) { build(:cluster_provider_gcp, :creating) } + + before do + gcp.make_created + end + + it 'nullify access_token and operation_id' do + expect(gcp.access_token).to be_nil + expect(gcp.operation_id).to be_nil + expect(gcp).to be_created + end + end + + context 'when any => [:creating]' do + let(:gcp) { build(:cluster_provider_gcp) } + + context 'when operation_id is present' do + let(:operation_id) { 'operation-xxx' } + + before do + gcp.make_creating(operation_id) + end + + it 'sets operation_id' do + expect(gcp.operation_id).to eq(operation_id) + expect(gcp).to be_creating + end + end + + context 'when operation_id is nil' do + let(:operation_id) { nil } + + it 'raises an error' do + expect { gcp.make_creating(operation_id) } + .to raise_error('operation_id is required') + end + end + end + + context 'when any => [:errored]' do + let(:gcp) { build(:cluster_provider_gcp, :creating) } + let(:status_reason) { 'err msg' } + + it 'nullify access_token and operation_id' do + gcp.make_errored(status_reason) + + expect(gcp.access_token).to be_nil + expect(gcp.operation_id).to be_nil + expect(gcp.status_reason).to eq(status_reason) + expect(gcp).to be_errored + end + + context 'when status_reason is nil' do + let(:gcp) { build(:cluster_provider_gcp, :errored) } + + it 'does not set status_reason' do + gcp.make_errored(nil) + + expect(gcp.status_reason).not_to be_nil + end + end + end + end + + describe '#on_creation?' do + subject { gcp.on_creation? } + + context 'when status is creating' do + let(:gcp) { create(:cluster_provider_gcp, :creating) } + + it { is_expected.to be_truthy } + end + + context 'when status is created' do + let(:gcp) { create(:cluster_provider_gcp, :created) } + + it { is_expected.to be_falsey } + end + end + + describe '#api_client' do + subject { gcp.api_client } + + context 'when status is creating' do + let(:gcp) { build(:cluster_provider_gcp, :creating) } + + it 'returns Cloud Platform API clinet' do + expect(subject).to be_an_instance_of(GoogleApi::CloudPlatform::Client) + expect(subject.access_token).to eq(gcp.access_token) + end + end + + context 'when status is created' do + let(:gcp) { build(:cluster_provider_gcp, :created) } + + it { is_expected.to be_nil } + end + + context 'when status is errored' do + let(:gcp) { build(:cluster_provider_gcp, :errored) } + + it { is_expected.to be_nil } + end + end +end diff --git a/spec/models/concerns/ignorable_column_spec.rb b/spec/models/concerns/ignorable_column_spec.rb index dba9fe43327..b70f2331a0e 100644 --- a/spec/models/concerns/ignorable_column_spec.rb +++ b/spec/models/concerns/ignorable_column_spec.rb @@ -5,7 +5,11 @@ describe IgnorableColumn do Class.new do def self.columns # This method does not have access to "double" - [Struct.new(:name).new('id'), Struct.new(:name).new('title')] + [ + Struct.new(:name).new('id'), + Struct.new(:name).new('title'), + Struct.new(:name).new('date') + ] end end end @@ -18,7 +22,7 @@ describe IgnorableColumn do describe '.columns' do it 'returns the columns, excluding the ignored ones' do - model.ignore_column(:title) + model.ignore_column(:title, :date) expect(model.columns.map(&:name)).to eq(%w(id)) end @@ -30,9 +34,9 @@ describe IgnorableColumn do end it 'returns the names of the ignored columns' do - model.ignore_column(:title) + model.ignore_column(:title, :date) - expect(model.ignored_columns).to eq(Set.new(%w(title))) + expect(model.ignored_columns).to eq(Set.new(%w(title date))) end end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index f75de0a0d88..1ce1d595c60 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -18,7 +18,6 @@ describe Environment do it { is_expected.to validate_length_of(:slug).is_at_most(24) } it { is_expected.to validate_length_of(:external_url).is_at_most(255) } - it { is_expected.to validate_uniqueness_of(:external_url).scoped_to(:project_id) } describe '.order_by_last_deployed_at' do let(:project) { create(:project, :repository) } diff --git a/spec/models/gcp/cluster_spec.rb b/spec/models/gcp/cluster_spec.rb deleted file mode 100644 index 8f39fff6394..00000000000 --- a/spec/models/gcp/cluster_spec.rb +++ /dev/null @@ -1,264 +0,0 @@ -require 'spec_helper' - -describe Gcp::Cluster do - it { is_expected.to belong_to(:project) } - it { is_expected.to belong_to(:user) } - it { is_expected.to belong_to(:service) } - - it { is_expected.to validate_presence_of(:gcp_cluster_zone) } - - describe '.enabled' do - subject { described_class.enabled } - - let!(:cluster) { create(:gcp_cluster, enabled: true) } - - before do - create(:gcp_cluster, enabled: false) - end - - it { is_expected.to contain_exactly(cluster) } - end - - describe '.disabled' do - subject { described_class.disabled } - - let!(:cluster) { create(:gcp_cluster, enabled: false) } - - before do - create(:gcp_cluster, enabled: true) - end - - it { is_expected.to contain_exactly(cluster) } - end - - describe '#default_value_for' do - let(:cluster) { described_class.new } - - it { expect(cluster.gcp_cluster_zone).to eq('us-central1-a') } - it { expect(cluster.gcp_cluster_size).to eq(3) } - it { expect(cluster.gcp_machine_type).to eq('n1-standard-4') } - end - - describe '#validates' do - subject { cluster.valid? } - - context 'when validates gcp_project_id' do - let(:cluster) { build(:gcp_cluster, gcp_project_id: gcp_project_id) } - - context 'when valid' do - let(:gcp_project_id) { 'gcp-project-12345' } - - it { is_expected.to be_truthy } - end - - context 'when empty' do - let(:gcp_project_id) { '' } - - it { is_expected.to be_falsey } - end - - context 'when too long' do - let(:gcp_project_id) { 'A' * 64 } - - it { is_expected.to be_falsey } - end - - context 'when includes abnormal character' do - let(:gcp_project_id) { '!!!!!!' } - - it { is_expected.to be_falsey } - end - end - - context 'when validates gcp_cluster_name' do - let(:cluster) { build(:gcp_cluster, gcp_cluster_name: gcp_cluster_name) } - - context 'when valid' do - let(:gcp_cluster_name) { 'test-cluster' } - - it { is_expected.to be_truthy } - end - - context 'when empty' do - let(:gcp_cluster_name) { '' } - - it { is_expected.to be_falsey } - end - - context 'when too long' do - let(:gcp_cluster_name) { 'A' * 64 } - - it { is_expected.to be_falsey } - end - - context 'when includes abnormal character' do - let(:gcp_cluster_name) { '!!!!!!' } - - it { is_expected.to be_falsey } - end - end - - context 'when validates gcp_cluster_size' do - let(:cluster) { build(:gcp_cluster, gcp_cluster_size: gcp_cluster_size) } - - context 'when valid' do - let(:gcp_cluster_size) { 1 } - - it { is_expected.to be_truthy } - end - - context 'when zero' do - let(:gcp_cluster_size) { 0 } - - it { is_expected.to be_falsey } - end - end - - context 'when validates project_namespace' do - let(:cluster) { build(:gcp_cluster, project_namespace: project_namespace) } - - context 'when valid' do - let(:project_namespace) { 'default-namespace' } - - it { is_expected.to be_truthy } - end - - context 'when empty' do - let(:project_namespace) { '' } - - it { is_expected.to be_truthy } - end - - context 'when too long' do - let(:project_namespace) { 'A' * 64 } - - it { is_expected.to be_falsey } - end - - context 'when includes abnormal character' do - let(:project_namespace) { '!!!!!!' } - - it { is_expected.to be_falsey } - end - end - - context 'when validates restrict_modification' do - let(:cluster) { create(:gcp_cluster) } - - before do - cluster.make_creating! - end - - context 'when created' do - before do - cluster.make_created! - end - - it { is_expected.to be_truthy } - end - - context 'when creating' do - it { is_expected.to be_falsey } - end - end - end - - describe '#state_machine' do - let(:cluster) { build(:gcp_cluster) } - - context 'when transits to created state' do - before do - cluster.gcp_token = 'tmp' - cluster.gcp_operation_id = 'tmp' - cluster.make_created! - end - - it 'nullify gcp_token and gcp_operation_id' do - expect(cluster.gcp_token).to be_nil - expect(cluster.gcp_operation_id).to be_nil - expect(cluster).to be_created - end - end - - context 'when transits to errored state' do - let(:reason) { 'something wrong' } - - before do - cluster.make_errored!(reason) - end - - it 'sets status_reason' do - expect(cluster.status_reason).to eq(reason) - expect(cluster).to be_errored - end - end - end - - describe '#project_namespace_placeholder' do - subject { cluster.project_namespace_placeholder } - - let(:cluster) { create(:gcp_cluster) } - - it 'returns a placeholder' do - is_expected.to eq("#{cluster.project.path}-#{cluster.project.id}") - end - end - - describe '#on_creation?' do - subject { cluster.on_creation? } - - let(:cluster) { create(:gcp_cluster) } - - context 'when status is creating' do - before do - cluster.make_creating! - end - - it { is_expected.to be_truthy } - end - - context 'when status is created' do - before do - cluster.make_created! - end - - it { is_expected.to be_falsey } - end - end - - describe '#api_url' do - subject { cluster.api_url } - - let(:cluster) { create(:gcp_cluster, :created_on_gke) } - let(:api_url) { 'https://' + cluster.endpoint } - - it { is_expected.to eq(api_url) } - end - - describe '#restrict_modification' do - subject { cluster.restrict_modification } - - let(:cluster) { create(:gcp_cluster) } - - context 'when status is created' do - before do - cluster.make_created! - end - - it { is_expected.to be_truthy } - end - - context 'when status is creating' do - before do - cluster.make_creating! - end - - it { is_expected.to be_falsey } - - it 'sets error' do - is_expected.to be_falsey - expect(cluster.errors).not_to be_empty - end - end - end -end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 476a2697605..d022dae3476 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1755,39 +1755,12 @@ describe MergeRequest do end end - describe '#fetch_ref' do - it 'sets "ref_fetched" flag to true' do - subject.update!(ref_fetched: nil) + describe '#fetch_ref!' do + it 'fetches the ref correctly' do + expect { subject.target_project.repository.delete_refs(subject.ref_path) }.not_to raise_error - subject.fetch_ref - - expect(subject.reload.ref_fetched).to be_truthy - end - end - - describe '#ref_fetched?' do - it 'does not perform git operation when value is cached' do - subject.ref_fetched = true - - expect_any_instance_of(Repository).not_to receive(:ref_exists?) - expect(subject.ref_fetched?).to be_truthy - end - - it 'caches the value when ref exists but value is not cached' do - subject.update!(ref_fetched: nil) - allow_any_instance_of(Repository).to receive(:ref_exists?) - .and_return(true) - - expect(subject.ref_fetched?).to be_truthy - expect(subject.reload.ref_fetched).to be_truthy - end - - it 'returns false when ref does not exist' do - subject.update!(ref_fetched: nil) - allow_any_instance_of(Repository).to receive(:ref_exists?) - .and_return(false) - - expect(subject.ref_fetched?).to be_falsey + subject.fetch_ref! + expect(subject.target_project.repository.ref_exists?(subject.ref_path)).to be_truthy end end diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb index 00de536a18b..7617e1f89b1 100644 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ b/spec/models/project_services/kubernetes_service_spec.rb @@ -145,7 +145,7 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do let(:discovery_url) { 'https://kubernetes.example.com/api/v1' } before do - stub_kubeclient_discover + stub_kubeclient_discover(service.api_url) end context 'with path prefix in api_url' do @@ -153,7 +153,7 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do it 'tests with the prefix' do service.api_url = 'https://kubernetes.example.com/prefix' - stub_kubeclient_discover + stub_kubeclient_discover(service.api_url) expect(service.test[:success]).to be_truthy expect(WebMock).to have_requested(:get, discovery_url).once diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e8588975118..0e50909988b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -276,6 +276,12 @@ describe Project do expect(project).to be_valid end + + it 'allows a path ending in a period' do + project = build(:project, path: 'foo.') + + expect(project).to be_valid + end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index d7c07676911..8a6aa767ce6 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2298,4 +2298,24 @@ describe Repository do project.commit_by(oid: '1' * 40) end end + + describe '#raw_repository' do + subject { repository.raw_repository } + + it 'returns a Gitlab::Git::Repository representation of the repository' do + expect(subject).to be_a(Gitlab::Git::Repository) + expect(subject.relative_path).to eq(project.disk_path + '.git') + expect(subject.gl_repository).to eq("project-#{project.id}") + end + + context 'with a wiki repository' do + let(:repository) { project.wiki.repository } + + it 'creates a Gitlab::Git::Repository with the proper attributes' do + expect(subject).to be_a(Gitlab::Git::Repository) + expect(subject.relative_path).to eq(project.disk_path + '.wiki.git') + expect(subject.gl_repository).to eq("wiki-#{project.id}") + end + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e0896d64c8f..d2f97009ad9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -788,14 +788,16 @@ describe User do end it "creates external user by default" do - user = build(:user) + user = create(:user) expect(user.external).to be_truthy + expect(user.can_create_group).to be_falsey + expect(user.projects_limit).to be 0 end describe 'with default overrides' do it "creates a non-external user" do - user = build(:user, external: false) + user = create(:user, external: false) expect(user.external).to be_falsey end diff --git a/spec/policies/gcp/cluster_policy_spec.rb b/spec/policies/clusters/cluster_policy_spec.rb index e213aa3d557..4207f42b07f 100644 --- a/spec/policies/gcp/cluster_policy_spec.rb +++ b/spec/policies/clusters/cluster_policy_spec.rb @@ -1,8 +1,8 @@ require 'spec_helper' -describe Gcp::ClusterPolicy, :models do - set(:project) { create(:project) } - set(:cluster) { create(:gcp_cluster, project: project) } +describe Clusters::ClusterPolicy, :models do + let(:cluster) { create(:cluster, :project) } + let(:project) { cluster.project } let(:user) { create(:user) } let(:policy) { described_class.new(user, cluster) } diff --git a/spec/presenters/gcp/cluster_presenter_spec.rb b/spec/presenters/clusters/cluster_presenter_spec.rb index 8d86dc31582..48d4f3671c5 100644 --- a/spec/presenters/gcp/cluster_presenter_spec.rb +++ b/spec/presenters/clusters/cluster_presenter_spec.rb @@ -1,8 +1,7 @@ require 'spec_helper' -describe Gcp::ClusterPresenter do - let(:project) { create(:project) } - let(:cluster) { create(:gcp_cluster, project: project) } +describe Clusters::ClusterPresenter do + let(:cluster) { create(:cluster, :provided_by_gcp) } subject(:presenter) do described_class.new(cluster) @@ -22,14 +21,14 @@ describe Gcp::ClusterPresenter do end it 'forwards missing methods to cluster' do - expect(presenter.gcp_cluster_zone).to eq(cluster.gcp_cluster_zone) + expect(presenter.status).to eq(cluster.status) end end describe '#gke_cluster_url' do subject { described_class.new(cluster).gke_cluster_url } - it { is_expected.to include(cluster.gcp_cluster_zone) } - it { is_expected.to include(cluster.gcp_cluster_name) } + it { is_expected.to include(cluster.provider.zone) } + it { is_expected.to include(cluster.name) } end end diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 3b7b9c889e7..1765907c1b4 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -165,7 +165,17 @@ describe API::Jobs do context 'authorized user' do it 'returns specific job data' do expect(response).to have_gitlab_http_status(200) - expect(json_response['name']).to eq('test') + expect(json_response['id']).to eq(job.id) + expect(json_response['status']).to eq(job.status) + expect(json_response['stage']).to eq(job.stage) + expect(json_response['name']).to eq(job.name) + expect(json_response['ref']).to eq(job.ref) + expect(json_response['tag']).to eq(job.tag) + expect(json_response['coverage']).to eq(job.coverage) + expect(Time.parse(json_response['created_at'])).to be_like_time(job.created_at) + expect(Time.parse(json_response['started_at'])).to be_like_time(job.started_at) + expect(Time.parse(json_response['finished_at'])).to be_like_time(job.finished_at) + expect(json_response['duration']).to eq(job.duration) end it 'returns pipeline data' do diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 024cfe8b372..e16be3c46e1 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -623,8 +623,6 @@ describe API::MergeRequests do before do forked_project.add_reporter(user2) - - allow_any_instance_of(MergeRequest).to receive(:write_ref) end it "returns merge_request" do diff --git a/spec/requests/api/v3/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb index 26251b95680..91897e5ee01 100644 --- a/spec/requests/api/v3/merge_requests_spec.rb +++ b/spec/requests/api/v3/merge_requests_spec.rb @@ -319,8 +319,6 @@ describe API::MergeRequests do before do forked_project.add_reporter(user2) - - allow_any_instance_of(MergeRequest).to receive(:write_ref) end it "returns merge_request" do diff --git a/spec/serializers/cluster_entity_spec.rb b/spec/serializers/cluster_entity_spec.rb index 2c7f49974f1..72f02131211 100644 --- a/spec/serializers/cluster_entity_spec.rb +++ b/spec/serializers/cluster_entity_spec.rb @@ -1,22 +1,38 @@ require 'spec_helper' describe ClusterEntity do - set(:cluster) { create(:gcp_cluster, :errored) } - let(:request) { double('request') } + describe '#as_json' do + subject { described_class.new(cluster).as_json } - let(:entity) do - described_class.new(cluster) - end + context 'when provider type is gcp' do + let(:cluster) { create(:cluster, provider_type: :gcp, provider_gcp: provider) } - describe '#as_json' do - subject { entity.as_json } + context 'when status is creating' do + let(:provider) { create(:cluster_provider_gcp, :creating) } - it 'contains status' do - expect(subject[:status]).to eq(:errored) + it 'has corresponded data' do + expect(subject[:status]).to eq(:creating) + expect(subject[:status_reason]).to be_nil + end + end + + context 'when status is errored' do + let(:provider) { create(:cluster_provider_gcp, :errored) } + + it 'has corresponded data' do + expect(subject[:status]).to eq(:errored) + expect(subject[:status_reason]).to eq(provider.status_reason) + end + end end - it 'contains status reason' do - expect(subject[:status_reason]).to eq('general error') + context 'when provider type is user' do + let(:cluster) { create(:cluster, provider_type: :user) } + + it 'has nil' do + expect(subject[:status]).to be_nil + expect(subject[:status_reason]).to be_nil + end end end end diff --git a/spec/serializers/cluster_serializer_spec.rb b/spec/serializers/cluster_serializer_spec.rb index 1ac6784d28f..ff7d1789149 100644 --- a/spec/serializers/cluster_serializer_spec.rb +++ b/spec/serializers/cluster_serializer_spec.rb @@ -1,15 +1,20 @@ require 'spec_helper' describe ClusterSerializer do - let(:serializer) do - described_class.new - end - describe '#represent_status' do - subject { serializer.represent_status(resource) } + subject { described_class.new.represent_status(cluster) } + + context 'when provider type is gcp' do + let(:cluster) { create(:cluster, provider_type: :gcp, provider_gcp: provider) } + let(:provider) { create(:cluster_provider_gcp, :errored) } + + it 'serializes only status' do + expect(subject.keys).to contain_exactly(:status, :status_reason) + end + end - context 'when represents only status' do - let(:resource) { create(:gcp_cluster, :errored) } + context 'when provider type is user' do + let(:cluster) { create(:cluster, provider_type: :user) } it 'serializes only status' do expect(subject.keys).to contain_exactly(:status, :status_reason) diff --git a/spec/services/ci/create_cluster_service_spec.rb b/spec/services/ci/create_cluster_service_spec.rb deleted file mode 100644 index 6e7398fbffa..00000000000 --- a/spec/services/ci/create_cluster_service_spec.rb +++ /dev/null @@ -1,47 +0,0 @@ -require 'spec_helper' - -describe Ci::CreateClusterService do - describe '#execute' do - let(:access_token) { 'xxx' } - let(:project) { create(:project) } - let(:user) { create(:user) } - let(:result) { described_class.new(project, user, params).execute(access_token) } - - context 'when correct params' do - let(:params) do - { - gcp_project_id: 'gcp-project', - gcp_cluster_name: 'test-cluster', - gcp_cluster_zone: 'us-central1-a', - gcp_cluster_size: 1 - } - end - - it 'creates a cluster object' do - expect(ClusterProvisionWorker).to receive(:perform_async) - expect { result }.to change { Gcp::Cluster.count }.by(1) - expect(result.gcp_project_id).to eq('gcp-project') - expect(result.gcp_cluster_name).to eq('test-cluster') - expect(result.gcp_cluster_zone).to eq('us-central1-a') - expect(result.gcp_cluster_size).to eq(1) - expect(result.gcp_token).to eq(access_token) - end - end - - context 'when invalid params' do - let(:params) do - { - gcp_project_id: 'gcp-project', - gcp_cluster_name: 'test-cluster', - gcp_cluster_zone: 'us-central1-a', - gcp_cluster_size: 'ABC' - } - end - - it 'returns an error' do - expect(ClusterProvisionWorker).not_to receive(:perform_async) - expect { result }.to change { Gcp::Cluster.count }.by(0) - end - end - end -end diff --git a/spec/services/ci/fetch_gcp_operation_service_spec.rb b/spec/services/ci/fetch_gcp_operation_service_spec.rb deleted file mode 100644 index 7792979c5cb..00000000000 --- a/spec/services/ci/fetch_gcp_operation_service_spec.rb +++ /dev/null @@ -1,36 +0,0 @@ -require 'spec_helper' -require 'google/apis' - -describe Ci::FetchGcpOperationService do - describe '#execute' do - let(:cluster) { create(:gcp_cluster) } - let(:operation) { double } - - context 'when suceeded' do - before do - allow_any_instance_of(GoogleApi::CloudPlatform::Client) - .to receive(:projects_zones_operations).and_return(operation) - end - - it 'fetch the gcp operaion' do - expect { |b| described_class.new.execute(cluster, &b) } - .to yield_with_args(operation) - end - end - - context 'when raises an error' do - let(:error) { Google::Apis::ServerError.new('a') } - - before do - allow_any_instance_of(GoogleApi::CloudPlatform::Client) - .to receive(:projects_zones_operations).and_raise(error) - end - - it 'sets an error to cluster object' do - expect { |b| described_class.new.execute(cluster, &b) } - .not_to yield_with_args - expect(cluster.reload).to be_errored - end - end - end -end diff --git a/spec/services/ci/finalize_cluster_creation_service_spec.rb b/spec/services/ci/finalize_cluster_creation_service_spec.rb deleted file mode 100644 index def3709fdb4..00000000000 --- a/spec/services/ci/finalize_cluster_creation_service_spec.rb +++ /dev/null @@ -1,61 +0,0 @@ -require 'spec_helper' - -describe Ci::FinalizeClusterCreationService do - describe '#execute' do - let(:cluster) { create(:gcp_cluster) } - let(:result) { described_class.new.execute(cluster) } - - context 'when suceeded to get cluster from api' do - let(:gke_cluster) { double } - - before do - allow(gke_cluster).to receive(:endpoint).and_return('111.111.111.111') - allow(gke_cluster).to receive(:master_auth).and_return(spy) - allow_any_instance_of(GoogleApi::CloudPlatform::Client) - .to receive(:projects_zones_clusters_get).and_return(gke_cluster) - end - - context 'when suceeded to get kubernetes token' do - let(:kubernetes_token) { 'abc' } - - before do - allow_any_instance_of(Ci::FetchKubernetesTokenService) - .to receive(:execute).and_return(kubernetes_token) - end - - it 'executes integration cluster' do - expect_any_instance_of(Ci::IntegrateClusterService).to receive(:execute) - described_class.new.execute(cluster) - end - end - - context 'when failed to get kubernetes token' do - before do - allow_any_instance_of(Ci::FetchKubernetesTokenService) - .to receive(:execute).and_return(nil) - end - - it 'sets an error to cluster object' do - described_class.new.execute(cluster) - - expect(cluster.reload).to be_errored - end - end - end - - context 'when failed to get cluster from api' do - let(:error) { Google::Apis::ServerError.new('a') } - - before do - allow_any_instance_of(GoogleApi::CloudPlatform::Client) - .to receive(:projects_zones_clusters_get).and_raise(error) - end - - it 'sets an error to cluster object' do - described_class.new.execute(cluster) - - expect(cluster.reload).to be_errored - end - end - end -end diff --git a/spec/services/ci/integrate_cluster_service_spec.rb b/spec/services/ci/integrate_cluster_service_spec.rb deleted file mode 100644 index 3a79c205bd1..00000000000 --- a/spec/services/ci/integrate_cluster_service_spec.rb +++ /dev/null @@ -1,42 +0,0 @@ -require 'spec_helper' - -describe Ci::IntegrateClusterService do - describe '#execute' do - let(:cluster) { create(:gcp_cluster, :custom_project_namespace) } - let(:endpoint) { '123.123.123.123' } - let(:ca_cert) { 'ca_cert_xxx' } - let(:token) { 'token_xxx' } - let(:username) { 'username_xxx' } - let(:password) { 'password_xxx' } - - before do - described_class - .new.execute(cluster, endpoint, ca_cert, token, username, password) - - cluster.reload - end - - context 'when correct params' do - it 'creates a cluster object' do - expect(cluster.endpoint).to eq(endpoint) - expect(cluster.ca_cert).to eq(ca_cert) - expect(cluster.kubernetes_token).to eq(token) - expect(cluster.username).to eq(username) - expect(cluster.password).to eq(password) - expect(cluster.service.active).to be_truthy - expect(cluster.service.api_url).to eq(cluster.api_url) - expect(cluster.service.ca_pem).to eq(ca_cert) - expect(cluster.service.namespace).to eq(cluster.project_namespace) - expect(cluster.service.token).to eq(token) - end - end - - context 'when invalid params' do - let(:endpoint) { nil } - - it 'sets an error to cluster object' do - expect(cluster).to be_errored - end - end - end -end diff --git a/spec/services/ci/provision_cluster_service_spec.rb b/spec/services/ci/provision_cluster_service_spec.rb deleted file mode 100644 index 5ce5c788314..00000000000 --- a/spec/services/ci/provision_cluster_service_spec.rb +++ /dev/null @@ -1,85 +0,0 @@ -require 'spec_helper' - -describe Ci::ProvisionClusterService do - describe '#execute' do - let(:cluster) { create(:gcp_cluster) } - let(:operation) { spy } - - shared_examples 'error' do - it 'sets an error to cluster object' do - described_class.new.execute(cluster) - - expect(cluster.reload).to be_errored - end - end - - context 'when suceeded to request provision' do - before do - allow_any_instance_of(GoogleApi::CloudPlatform::Client) - .to receive(:projects_zones_clusters_create).and_return(operation) - end - - context 'when operation status is RUNNING' do - before do - allow(operation).to receive(:status).and_return('RUNNING') - end - - context 'when suceeded to parse gcp operation id' do - before do - allow_any_instance_of(GoogleApi::CloudPlatform::Client) - .to receive(:parse_operation_id).and_return('operation-123') - end - - context 'when cluster status is scheduled' do - before do - allow_any_instance_of(GoogleApi::CloudPlatform::Client) - .to receive(:parse_operation_id).and_return('operation-123') - end - - it 'schedules a worker for status minitoring' do - expect(WaitForClusterCreationWorker).to receive(:perform_in) - - described_class.new.execute(cluster) - end - end - - context 'when cluster status is creating' do - before do - cluster.make_creating! - end - - it_behaves_like 'error' - end - end - - context 'when failed to parse gcp operation id' do - before do - allow_any_instance_of(GoogleApi::CloudPlatform::Client) - .to receive(:parse_operation_id).and_return(nil) - end - - it_behaves_like 'error' - end - end - - context 'when operation status is others' do - before do - allow(operation).to receive(:status).and_return('others') - end - - it_behaves_like 'error' - end - end - - context 'when failed to request provision' do - let(:error) { Google::Apis::ServerError.new('a') } - - before do - allow_any_instance_of(GoogleApi::CloudPlatform::Client) - .to receive(:projects_zones_clusters_create).and_raise(error) - end - - it_behaves_like 'error' - end - end -end diff --git a/spec/services/ci/update_cluster_service_spec.rb b/spec/services/ci/update_cluster_service_spec.rb deleted file mode 100644 index a289385b88f..00000000000 --- a/spec/services/ci/update_cluster_service_spec.rb +++ /dev/null @@ -1,37 +0,0 @@ -require 'spec_helper' - -describe Ci::UpdateClusterService do - describe '#execute' do - let(:cluster) { create(:gcp_cluster, :created_on_gke, :with_kubernetes_service) } - - before do - described_class.new(cluster.project, cluster.user, params).execute(cluster) - - cluster.reload - end - - context 'when correct params' do - context 'when enabled is true' do - let(:params) { { 'enabled' => 'true' } } - - it 'enables cluster and overwrite kubernetes service' do - expect(cluster.enabled).to be_truthy - expect(cluster.service.active).to be_truthy - expect(cluster.service.api_url).to eq(cluster.api_url) - expect(cluster.service.ca_pem).to eq(cluster.ca_cert) - expect(cluster.service.namespace).to eq(cluster.project_namespace) - expect(cluster.service.token).to eq(cluster.kubernetes_token) - end - end - - context 'when enabled is false' do - let(:params) { { 'enabled' => 'false' } } - - it 'disables cluster and kubernetes service' do - expect(cluster.enabled).to be_falsy - expect(cluster.service.active).to be_falsy - end - end - end - end -end diff --git a/spec/services/clusters/create_service_spec.rb b/spec/services/clusters/create_service_spec.rb new file mode 100644 index 00000000000..5b6edb73beb --- /dev/null +++ b/spec/services/clusters/create_service_spec.rb @@ -0,0 +1,64 @@ +require 'spec_helper' + +describe Clusters::CreateService do + let(:access_token) { 'xxx' } + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:result) { described_class.new(project, user, params).execute(access_token) } + + context 'when provider is gcp' do + context 'when correct params' do + let(:params) do + { + name: 'test-cluster', + provider_type: :gcp, + provider_gcp_attributes: { + gcp_project_id: 'gcp-project', + zone: 'us-central1-a', + num_nodes: 1, + machine_type: 'machine_type-a' + } + } + end + + it 'creates a cluster object and performs a worker' do + expect(ClusterProvisionWorker).to receive(:perform_async) + + expect { result } + .to change { Clusters::Cluster.count }.by(1) + .and change { Clusters::Providers::Gcp.count }.by(1) + + expect(result.name).to eq('test-cluster') + expect(result.user).to eq(user) + expect(result.project).to eq(project) + expect(result.provider.gcp_project_id).to eq('gcp-project') + expect(result.provider.zone).to eq('us-central1-a') + expect(result.provider.num_nodes).to eq(1) + expect(result.provider.machine_type).to eq('machine_type-a') + expect(result.provider.access_token).to eq(access_token) + expect(result.platform).to be_nil + end + end + + context 'when invalid params' do + let(:params) do + { + name: 'test-cluster', + provider_type: :gcp, + provider_gcp_attributes: { + gcp_project_id: '!!!!!!!', + zone: 'us-central1-a', + num_nodes: 1, + machine_type: 'machine_type-a' + } + } + end + + it 'returns an error' do + expect(ClusterProvisionWorker).not_to receive(:perform_async) + expect { result }.to change { Clusters::Cluster.count }.by(0) + expect(result.errors[:"provider_gcp.gcp_project_id"]).to be_present + end + end + end +end diff --git a/spec/services/clusters/gcp/fetch_operation_service_spec.rb b/spec/services/clusters/gcp/fetch_operation_service_spec.rb new file mode 100644 index 00000000000..e2fa93904c5 --- /dev/null +++ b/spec/services/clusters/gcp/fetch_operation_service_spec.rb @@ -0,0 +1,43 @@ +require 'spec_helper' + +describe Clusters::Gcp::FetchOperationService do + include GoogleApi::CloudPlatformHelpers + + describe '#execute' do + let(:provider) { create(:cluster_provider_gcp, :creating) } + let(:gcp_project_id) { provider.gcp_project_id } + let(:zone) { provider.zone } + let(:operation_id) { provider.operation_id } + + shared_examples 'success' do + it 'yields' do + expect { |b| described_class.new.execute(provider, &b) } + .to yield_with_args + end + end + + shared_examples 'error' do + it 'sets an error to provider object' do + expect { |b| described_class.new.execute(provider, &b) } + .not_to yield_with_args + expect(provider.reload).to be_errored + end + end + + context 'when suceeded to fetch operation' do + before do + stub_cloud_platform_get_zone_operation(gcp_project_id, zone, operation_id) + end + + it_behaves_like 'success' + end + + context 'when Internal Server Error happened' do + before do + stub_cloud_platform_get_zone_operation_error(gcp_project_id, zone, operation_id) + end + + it_behaves_like 'error' + end + end +end diff --git a/spec/services/clusters/gcp/finalize_creation_service_spec.rb b/spec/services/clusters/gcp/finalize_creation_service_spec.rb new file mode 100644 index 00000000000..0cf91307589 --- /dev/null +++ b/spec/services/clusters/gcp/finalize_creation_service_spec.rb @@ -0,0 +1,111 @@ +require 'spec_helper' + +describe Clusters::Gcp::FinalizeCreationService do + include GoogleApi::CloudPlatformHelpers + include KubernetesHelpers + + describe '#execute' do + let(:cluster) { create(:cluster, :project, :providing_by_gcp) } + let(:provider) { cluster.provider } + let(:platform) { cluster.platform } + let(:gcp_project_id) { provider.gcp_project_id } + let(:zone) { provider.zone } + let(:cluster_name) { cluster.name } + + shared_examples 'success' do + it 'configures provider and kubernetes' do + described_class.new.execute(provider) + + expect(provider).to be_created + end + end + + shared_examples 'error' do + it 'sets an error to provider object' do + described_class.new.execute(provider) + + expect(provider.reload).to be_errored + end + end + + context 'when suceeded to fetch gke cluster info' do + let(:endpoint) { '111.111.111.111' } + let(:api_url) { 'https://' + endpoint } + let(:username) { 'sample-username' } + let(:password) { 'sample-password' } + + before do + stub_cloud_platform_get_zone_cluster( + gcp_project_id, zone, cluster_name, + { + endpoint: endpoint, + username: username, + password: password + } + ) + + stub_kubeclient_discover(api_url) + end + + context 'when suceeded to fetch kuberenetes token' do + let(:token) { 'sample-token' } + + before do + stub_kubeclient_get_secrets( + api_url, + { + token: Base64.encode64(token) + } ) + end + + it_behaves_like 'success' + + it 'has corresponded data' do + described_class.new.execute(provider) + cluster.reload + provider.reload + platform.reload + + expect(provider.endpoint).to eq(endpoint) + expect(platform.api_url).to eq(api_url) + expect(platform.ca_cert).to eq(Base64.decode64(load_sample_cert)) + expect(platform.username).to eq(username) + expect(platform.password).to eq(password) + expect(platform.token).to eq(token) + end + end + + context 'when default-token is not found' do + before do + stub_kubeclient_get_secrets(api_url, metadata_name: 'aaaa') + end + + it_behaves_like 'error' + end + + context 'when token is empty' do + before do + stub_kubeclient_get_secrets(api_url, token: '') + end + + it_behaves_like 'error' + end + + context 'when failed to fetch kuberenetes token' do + before do + stub_kubeclient_get_secrets_error(api_url) + end + + it_behaves_like 'error' + end + end + + context 'when failed to fetch gke cluster info' do + before do + stub_cloud_platform_get_zone_cluster_error(gcp_project_id, zone, cluster_name) + end + + it_behaves_like 'error' + end + end +end diff --git a/spec/services/clusters/gcp/provision_service_spec.rb b/spec/services/clusters/gcp/provision_service_spec.rb new file mode 100644 index 00000000000..f48afdc83b2 --- /dev/null +++ b/spec/services/clusters/gcp/provision_service_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +describe Clusters::Gcp::ProvisionService do + include GoogleApi::CloudPlatformHelpers + + describe '#execute' do + let(:provider) { create(:cluster_provider_gcp, :scheduled) } + let(:gcp_project_id) { provider.gcp_project_id } + let(:zone) { provider.zone } + + shared_examples 'success' do + it 'schedules a worker for status minitoring' do + expect(WaitForClusterCreationWorker).to receive(:perform_in) + + described_class.new.execute(provider) + + expect(provider.reload).to be_creating + end + end + + shared_examples 'error' do + it 'sets an error to provider object' do + described_class.new.execute(provider) + + expect(provider.reload).to be_errored + end + end + + context 'when suceeded to request provision' do + before do + stub_cloud_platform_create_cluster(gcp_project_id, zone) + end + + it_behaves_like 'success' + end + + context 'when operation status is unexpected' do + before do + stub_cloud_platform_create_cluster( + gcp_project_id, zone, + { + "status": 'unexpected' + } ) + end + + it_behaves_like 'error' + end + + context 'when selfLink is unexpected' do + before do + stub_cloud_platform_create_cluster( + gcp_project_id, zone, + { + "selfLink": 'unexpected' + }) + end + + it_behaves_like 'error' + end + + context 'when Internal Server Error happened' do + before do + stub_cloud_platform_create_cluster_error(gcp_project_id, zone) + end + + it_behaves_like 'error' + end + end +end diff --git a/spec/services/clusters/gcp/verify_provision_status_service_spec.rb b/spec/services/clusters/gcp/verify_provision_status_service_spec.rb new file mode 100644 index 00000000000..2ee2fa51f63 --- /dev/null +++ b/spec/services/clusters/gcp/verify_provision_status_service_spec.rb @@ -0,0 +1,107 @@ +require 'spec_helper' + +describe Clusters::Gcp::VerifyProvisionStatusService do + include GoogleApi::CloudPlatformHelpers + + describe '#execute' do + let(:provider) { create(:cluster_provider_gcp, :creating) } + let(:gcp_project_id) { provider.gcp_project_id } + let(:zone) { provider.zone } + let(:operation_id) { provider.operation_id } + + shared_examples 'continue_creation' do + it 'schedules a worker for status minitoring' do + expect(WaitForClusterCreationWorker).to receive(:perform_in) + + described_class.new.execute(provider) + end + end + + shared_examples 'finalize_creation' do + it 'schedules a worker for status minitoring' do + expect_any_instance_of(Clusters::Gcp::FinalizeCreationService).to receive(:execute) + + described_class.new.execute(provider) + end + end + + shared_examples 'error' do + it 'sets an error to provider object' do + described_class.new.execute(provider) + + expect(provider.reload).to be_errored + end + end + + context 'when operation status is RUNNING' do + before do + stub_cloud_platform_get_zone_operation( + gcp_project_id, zone, operation_id, + { + "status": 'RUNNING', + "startTime": 1.minute.ago.strftime("%FT%TZ") + } ) + end + + it_behaves_like 'continue_creation' + + context 'when cluster creation time exceeds timeout' do + before do + stub_cloud_platform_get_zone_operation( + gcp_project_id, zone, operation_id, + { + "status": 'RUNNING', + "startTime": 30.minutes.ago.strftime("%FT%TZ") + } ) + end + + it_behaves_like 'error' + end + end + + context 'when operation status is PENDING' do + before do + stub_cloud_platform_get_zone_operation( + gcp_project_id, zone, operation_id, + { + "status": 'PENDING', + "startTime": 1.minute.ago.strftime("%FT%TZ") + } ) + end + + it_behaves_like 'continue_creation' + end + + context 'when operation status is DONE' do + before do + stub_cloud_platform_get_zone_operation( + gcp_project_id, zone, operation_id, + { + "status": 'DONE' + } ) + end + + it_behaves_like 'finalize_creation' + end + + context 'when operation status is unexpected' do + before do + stub_cloud_platform_get_zone_operation( + gcp_project_id, zone, operation_id, + { + "status": 'unexpected' + } ) + end + + it_behaves_like 'error' + end + + context 'when failed to get operation status' do + before do + stub_cloud_platform_get_zone_operation_error(gcp_project_id, zone, operation_id) + end + + it_behaves_like 'error' + end + end +end diff --git a/spec/services/clusters/update_service_spec.rb b/spec/services/clusters/update_service_spec.rb new file mode 100644 index 00000000000..2d91a21035d --- /dev/null +++ b/spec/services/clusters/update_service_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +describe Clusters::UpdateService do + describe '#execute' do + subject { described_class.new(cluster.project, cluster.user, params).execute(cluster) } + + let(:cluster) { create(:cluster, :project, :provided_by_user) } + + context 'when correct params' do + context 'when enabled is true' do + let(:params) { { enabled: true } } + + it 'enables cluster' do + is_expected.to eq(true) + expect(cluster.enabled).to be_truthy + end + end + + context 'when enabled is false' do + let(:params) { { enabled: false } } + + it 'disables cluster' do + is_expected.to eq(true) + expect(cluster.enabled).to be_falsy + end + end + + context 'when namespace is specified' do + let(:params) do + { + platform_kubernetes_attributes: { + namespace: 'custom-namespace' + } + } + end + + it 'updates namespace' do + is_expected.to eq(true) + expect(cluster.platform.namespace).to eq('custom-namespace') + end + end + end + + context 'when invalid params' do + let(:params) do + { + platform_kubernetes_attributes: { + namespace: '!!!' + } + } + end + + it 'returns false' do + is_expected.to eq(false) + expect(cluster.errors[:"platform_kubernetes.namespace"]).to be_present + end + end + end +end diff --git a/spec/services/events/render_service_spec.rb b/spec/services/events/render_service_spec.rb new file mode 100644 index 00000000000..b4a4a44d07b --- /dev/null +++ b/spec/services/events/render_service_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe Events::RenderService do + describe '#execute' do + let!(:note) { build(:note) } + let!(:event) { build(:event, target: note, project: note.project) } + let!(:user) { build(:user) } + + context 'when the request format is atom' do + it 'renders the note inside events' do + expect(Banzai::ObjectRenderer).to receive(:new) + .with(event.project, user, + only_path: false, + xhtml: true) + .and_call_original + + expect_any_instance_of(Banzai::ObjectRenderer) + .to receive(:render).with([note], :note) + + described_class.new(user).execute([event], atom_request: true) + end + end + + context 'when the request format is not atom' do + it 'renders the note inside events' do + expect(Banzai::ObjectRenderer).to receive(:new) + .with(event.project, user, {}) + .and_call_original + + expect_any_instance_of(Banzai::ObjectRenderer) + .to receive(:render).with([note], :note) + + described_class.new(user).execute([event], atom_request: false) + end + end + end +end diff --git a/spec/services/notes/render_service_spec.rb b/spec/services/notes/render_service_spec.rb new file mode 100644 index 00000000000..faac498037f --- /dev/null +++ b/spec/services/notes/render_service_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Notes::RenderService do + describe '#execute' do + it 'renders a Note' do + note = double(:note) + project = double(:project) + wiki = double(:wiki) + user = double(:user) + + expect(Banzai::ObjectRenderer).to receive(:new) + .with(project, user, + requested_path: 'foo', + project_wiki: wiki, + ref: 'bar', + only_path: nil, + xhtml: false) + .and_call_original + + expect_any_instance_of(Banzai::ObjectRenderer) + .to receive(:render).with([note], :note) + + described_class.new(user).execute([note], project, + requested_path: 'foo', + project_wiki: wiki, + ref: 'bar', + only_path: nil, + xhtml: false) + end + end +end diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index a9b34a5258a..dc2673abc73 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -248,11 +248,11 @@ describe TodoService do end end - describe '#destroy_issue' do + describe '#destroy_issuable' do it 'refresh the todos count cache for the user' do expect(john_doe).to receive(:update_todos_count_cache).and_call_original - service.destroy_issue(issue, john_doe) + service.destroy_issuable(issue, john_doe) end end @@ -643,14 +643,6 @@ describe TodoService do end end - describe '#destroy_merge_request' do - it 'refresh the todos count cache for the user' do - expect(john_doe).to receive(:update_todos_count_cache).and_call_original - - service.destroy_merge_request(mr_assigned, john_doe) - end - end - describe '#reassigned_merge_request' do it 'creates a pending todo for new assignee' do mr_unassigned.update_attribute(:assignee, john_doe) diff --git a/spec/support/google_api/cloud_platform_helpers.rb b/spec/support/google_api/cloud_platform_helpers.rb new file mode 100644 index 00000000000..dabf0db7666 --- /dev/null +++ b/spec/support/google_api/cloud_platform_helpers.rb @@ -0,0 +1,119 @@ +module GoogleApi + module CloudPlatformHelpers + def stub_google_api_validate_token + request.session[GoogleApi::CloudPlatform::Client.session_key_for_token] = 'token' + request.session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] = 1.hour.since.to_i.to_s + end + + def stub_google_api_expired_token + request.session[GoogleApi::CloudPlatform::Client.session_key_for_token] = 'token' + request.session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] = 1.hour.ago.to_i.to_s + end + + def stub_cloud_platform_get_zone_cluster(project_id, zone, cluster_id, **options) + WebMock.stub_request(:get, cloud_platform_get_zone_cluster_url(project_id, zone, cluster_id)) + .to_return(cloud_platform_response(cloud_platform_cluster_body(options))) + end + + def stub_cloud_platform_get_zone_cluster_error(project_id, zone, cluster_id) + WebMock.stub_request(:get, cloud_platform_get_zone_cluster_url(project_id, zone, cluster_id)) + .to_return(status: [500, "Internal Server Error"]) + end + + def stub_cloud_platform_create_cluster(project_id, zone, **options) + WebMock.stub_request(:post, cloud_platform_create_cluster_url(project_id, zone)) + .to_return(cloud_platform_response(cloud_platform_operation_body(options))) + end + + def stub_cloud_platform_create_cluster_error(project_id, zone) + WebMock.stub_request(:post, cloud_platform_create_cluster_url(project_id, zone)) + .to_return(status: [500, "Internal Server Error"]) + end + + def stub_cloud_platform_get_zone_operation(project_id, zone, operation_id, **options) + WebMock.stub_request(:get, cloud_platform_get_zone_operation_url(project_id, zone, operation_id)) + .to_return(cloud_platform_response(cloud_platform_operation_body(options))) + end + + def stub_cloud_platform_get_zone_operation_error(project_id, zone, operation_id) + WebMock.stub_request(:get, cloud_platform_get_zone_operation_url(project_id, zone, operation_id)) + .to_return(status: [500, "Internal Server Error"]) + end + + def cloud_platform_get_zone_cluster_url(project_id, zone, cluster_id) + "https://container.googleapis.com/v1/projects/#{project_id}/zones/#{zone}/clusters/#{cluster_id}" + end + + def cloud_platform_create_cluster_url(project_id, zone) + "https://container.googleapis.com/v1/projects/#{project_id}/zones/#{zone}/clusters" + end + + def cloud_platform_get_zone_operation_url(project_id, zone, operation_id) + "https://container.googleapis.com/v1/projects/#{project_id}/zones/#{zone}/operations/#{operation_id}" + end + + def cloud_platform_response(body) + { status: 200, headers: { 'Content-Type' => 'application/json' }, body: body.to_json } + end + + def load_sample_cert + pem_file = File.expand_path(Rails.root.join('spec/fixtures/clusters/sample_cert.pem')) + Base64.encode64(File.read(pem_file)) + end + + ## + # gcloud container clusters create + # https://cloud.google.com/container-engine/reference/rest/v1/projects.zones.clusters/create + # rubocop:disable Metrics/CyclomaticComplexity + # rubocop:disable Metrics/PerceivedComplexity + def cloud_platform_cluster_body(**options) + { + "name": options[:name] || 'string', + "description": options[:description] || 'string', + "initialNodeCount": options[:initialNodeCount] || 'number', + "masterAuth": { + "username": options[:username] || 'string', + "password": options[:password] || 'string', + "clusterCaCertificate": options[:clusterCaCertificate] || load_sample_cert, + "clientCertificate": options[:clientCertificate] || 'string', + "clientKey": options[:clientKey] || 'string' + }, + "loggingService": options[:loggingService] || 'string', + "monitoringService": options[:monitoringService] || 'string', + "network": options[:network] || 'string', + "clusterIpv4Cidr": options[:clusterIpv4Cidr] || 'string', + "subnetwork": options[:subnetwork] || 'string', + "enableKubernetesAlpha": options[:enableKubernetesAlpha] || 'boolean', + "labelFingerprint": options[:labelFingerprint] || 'string', + "selfLink": options[:selfLink] || 'string', + "zone": options[:zone] || 'string', + "endpoint": options[:endpoint] || 'string', + "initialClusterVersion": options[:initialClusterVersion] || 'string', + "currentMasterVersion": options[:currentMasterVersion] || 'string', + "currentNodeVersion": options[:currentNodeVersion] || 'string', + "createTime": options[:createTime] || 'string', + "status": options[:status] || 'RUNNING', + "statusMessage": options[:statusMessage] || 'string', + "nodeIpv4CidrSize": options[:nodeIpv4CidrSize] || 'number', + "servicesIpv4Cidr": options[:servicesIpv4Cidr] || 'string', + "currentNodeCount": options[:currentNodeCount] || 'number', + "expireTime": options[:expireTime] || 'string' + } + end + + def cloud_platform_operation_body(**options) + { + "name": options[:name] || 'operation-1234567891234-1234567', + "zone": options[:zone] || 'us-central1-a', + "operationType": options[:operationType] || 'CREATE_CLUSTER', + "status": options[:status] || 'PENDING', + "detail": options[:detail] || 'detail', + "statusMessage": options[:statusMessage] || '', + "selfLink": options[:selfLink] || 'https://container.googleapis.com/v1/projects/123456789101/zones/us-central1-a/operations/operation-1234567891234-1234567', + "targetLink": options[:targetLink] || 'https://container.googleapis.com/v1/projects/123456789101/zones/us-central1-a/clusters/test-cluster', + "startTime": options[:startTime] || '2017-09-13T16:49:13.055601589Z', + "endTime": options[:endTime] || '' + } + end + end +end diff --git a/spec/support/kubernetes_helpers.rb b/spec/support/kubernetes_helpers.rb index c92f78b324c..e46b61b6461 100644 --- a/spec/support/kubernetes_helpers.rb +++ b/spec/support/kubernetes_helpers.rb @@ -9,22 +9,51 @@ module KubernetesHelpers kube_response(kube_pods_body) end - def stub_kubeclient_discover - WebMock.stub_request(:get, service.api_url + '/api/v1').to_return(kube_response(kube_v1_discovery_body)) + def stub_kubeclient_discover(api_url) + WebMock.stub_request(:get, api_url + '/api/v1').to_return(kube_response(kube_v1_discovery_body)) end def stub_kubeclient_pods(response = nil) - stub_kubeclient_discover + stub_kubeclient_discover(service.api_url) pods_url = service.api_url + "/api/v1/namespaces/#{service.actual_namespace}/pods" WebMock.stub_request(:get, pods_url).to_return(response || kube_pods_response) end + def stub_kubeclient_get_secrets(api_url, **options) + WebMock.stub_request(:get, api_url + '/api/v1/secrets') + .to_return(kube_response(kube_v1_secrets_body(options))) + end + + def stub_kubeclient_get_secrets_error(api_url) + WebMock.stub_request(:get, api_url + '/api/v1/secrets') + .to_return(status: [404, "Internal Server Error"]) + end + + def kube_v1_secrets_body(**options) + { + "kind" => "SecretList", + "apiVersion": "v1", + "items" => [ + { + "metadata": { + "name": options[:metadata_name] || "default-token-1", + "namespace": "kube-system" + }, + "data": { + "token": options[:token] || Base64.encode64('token-sample-123') + } + } + ] + } + end + def kube_v1_discovery_body { "kind" => "APIResourceList", "resources" => [ - { "name" => "pods", "namespaced" => true, "kind" => "Pod" } + { "name" => "pods", "namespaced" => true, "kind" => "Pod" }, + { "name" => "secrets", "namespaced" => true, "kind" => "Secret" } ] } end diff --git a/spec/validators/dynamic_path_validator_spec.rb b/spec/validators/dynamic_path_validator_spec.rb deleted file mode 100644 index 08e1c5a728a..00000000000 --- a/spec/validators/dynamic_path_validator_spec.rb +++ /dev/null @@ -1,97 +0,0 @@ -require 'spec_helper' - -describe DynamicPathValidator do - let(:validator) { described_class.new(attributes: [:path]) } - - def expect_handles_invalid_utf8 - expect { yield('\255invalid') }.to be_falsey - end - - describe '.valid_user_path' do - it 'handles invalid utf8' do - expect(described_class.valid_user_path?("a\0weird\255path")).to be_falsey - end - end - - describe '.valid_group_path' do - it 'handles invalid utf8' do - expect(described_class.valid_group_path?("a\0weird\255path")).to be_falsey - end - end - - describe '.valid_project_path' do - it 'handles invalid utf8' do - expect(described_class.valid_project_path?("a\0weird\255path")).to be_falsey - end - end - - describe '#path_valid_for_record?' do - context 'for project' do - it 'calls valid_project_path?' do - project = build(:project, path: 'activity') - - expect(described_class).to receive(:valid_project_path?).with(project.full_path).and_call_original - - expect(validator.path_valid_for_record?(project, 'activity')).to be_truthy - end - end - - context 'for group' do - it 'calls valid_group_path?' do - group = build(:group, :nested, path: 'activity') - - expect(described_class).to receive(:valid_group_path?).with(group.full_path).and_call_original - - expect(validator.path_valid_for_record?(group, 'activity')).to be_falsey - end - end - - context 'for user' do - it 'calls valid_user_path?' do - user = build(:user, username: 'activity') - - expect(described_class).to receive(:valid_user_path?).with(user.full_path).and_call_original - - expect(validator.path_valid_for_record?(user, 'activity')).to be_truthy - end - end - - context 'for user namespace' do - it 'calls valid_user_path?' do - user = create(:user, username: 'activity') - namespace = user.namespace - - expect(described_class).to receive(:valid_user_path?).with(namespace.full_path).and_call_original - - expect(validator.path_valid_for_record?(namespace, 'activity')).to be_truthy - end - end - end - - describe '#validates_each' do - it 'adds a message when the path is not in the correct format' do - group = build(:group) - - validator.validate_each(group, :path, "Path with spaces, and comma's!") - - expect(group.errors[:path]).to include(Gitlab::PathRegex.namespace_format_message) - end - - it 'adds a message when the path is not in the correct format' do - group = build(:group, path: 'users') - - validator.validate_each(group, :path, 'users') - - expect(group.errors[:path]).to include('users is a reserved name') - end - - it 'updating to an invalid path is not allowed' do - project = create(:project) - project.path = 'update' - - validator.validate_each(project, :path, 'update') - - expect(project.errors[:path]).to include('update is a reserved name') - end - end -end diff --git a/spec/validators/namespace_path_validator_spec.rb b/spec/validators/namespace_path_validator_spec.rb new file mode 100644 index 00000000000..61e2845f35f --- /dev/null +++ b/spec/validators/namespace_path_validator_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe NamespacePathValidator do + let(:validator) { described_class.new(attributes: [:path]) } + + describe '.valid_path?' do + it 'handles invalid utf8' do + expect(described_class.valid_path?("a\0weird\255path")).to be_falsey + end + end + + describe '#validates_each' do + it 'adds a message when the path is not in the correct format' do + group = build(:group) + + validator.validate_each(group, :path, "Path with spaces, and comma's!") + + expect(group.errors[:path]).to include(Gitlab::PathRegex.namespace_format_message) + end + + it 'adds a message when the path is reserved when creating' do + group = build(:group, path: 'help') + + validator.validate_each(group, :path, 'help') + + expect(group.errors[:path]).to include('help is a reserved name') + end + + it 'adds a message when the path is reserved when updating' do + group = create(:group) + group.path = 'help' + + validator.validate_each(group, :path, 'help') + + expect(group.errors[:path]).to include('help is a reserved name') + end + end +end diff --git a/spec/validators/project_path_validator_spec.rb b/spec/validators/project_path_validator_spec.rb new file mode 100644 index 00000000000..8bb5e72dc22 --- /dev/null +++ b/spec/validators/project_path_validator_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe ProjectPathValidator do + let(:validator) { described_class.new(attributes: [:path]) } + + describe '.valid_path?' do + it 'handles invalid utf8' do + expect(described_class.valid_path?("a\0weird\255path")).to be_falsey + end + end + + describe '#validates_each' do + it 'adds a message when the path is not in the correct format' do + project = build(:project) + + validator.validate_each(project, :path, "Path with spaces, and comma's!") + + expect(project.errors[:path]).to include(Gitlab::PathRegex.project_path_format_message) + end + + it 'adds a message when the path is reserved when creating' do + project = build(:project, path: 'blob') + + validator.validate_each(project, :path, 'blob') + + expect(project.errors[:path]).to include('blob is a reserved name') + end + + it 'adds a message when the path is reserved when updating' do + project = create(:project) + project.path = 'blob' + + validator.validate_each(project, :path, 'blob') + + expect(project.errors[:path]).to include('blob is a reserved name') + end + end +end diff --git a/spec/validators/user_path_validator_spec.rb b/spec/validators/user_path_validator_spec.rb new file mode 100644 index 00000000000..a46089cc24f --- /dev/null +++ b/spec/validators/user_path_validator_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe UserPathValidator do + let(:validator) { described_class.new(attributes: [:username]) } + + describe '.valid_path?' do + it 'handles invalid utf8' do + expect(described_class.valid_path?("a\0weird\255path")).to be_falsey + end + end + + describe '#validates_each' do + it 'adds a message when the path is not in the correct format' do + user = build(:user) + + validator.validate_each(user, :username, "Path with spaces, and comma's!") + + expect(user.errors[:username]).to include(Gitlab::PathRegex.namespace_format_message) + end + + it 'adds a message when the path is reserved when creating' do + user = build(:user, username: 'help') + + validator.validate_each(user, :username, 'help') + + expect(user.errors[:username]).to include('help is a reserved name') + end + + it 'adds a message when the path is reserved when updating' do + user = create(:user) + user.username = 'help' + + validator.validate_each(user, :username, 'help') + + expect(user.errors[:username]).to include('help is a reserved name') + end + end +end diff --git a/spec/workers/cluster_provision_worker_spec.rb b/spec/workers/cluster_provision_worker_spec.rb index 11f208289db..8054ec11a48 100644 --- a/spec/workers/cluster_provision_worker_spec.rb +++ b/spec/workers/cluster_provision_worker_spec.rb @@ -2,11 +2,22 @@ require 'spec_helper' describe ClusterProvisionWorker do describe '#perform' do - context 'when cluster exists' do - let(:cluster) { create(:gcp_cluster) } + context 'when provider type is gcp' do + let(:cluster) { create(:cluster, provider_type: :gcp, provider_gcp: provider) } + let(:provider) { create(:cluster_provider_gcp, :scheduled) } it 'provision a cluster' do - expect_any_instance_of(Ci::ProvisionClusterService).to receive(:execute) + expect_any_instance_of(Clusters::Gcp::ProvisionService).to receive(:execute) + + described_class.new.perform(cluster.id) + end + end + + context 'when provider type is user' do + let(:cluster) { create(:cluster, provider_type: :user) } + + it 'does not provision a cluster' do + expect_any_instance_of(Clusters::Gcp::ProvisionService).not_to receive(:execute) described_class.new.perform(cluster.id) end @@ -14,7 +25,7 @@ describe ClusterProvisionWorker do context 'when cluster does not exist' do it 'does not provision a cluster' do - expect_any_instance_of(Ci::ProvisionClusterService).not_to receive(:execute) + expect_any_instance_of(Clusters::Gcp::ProvisionService).not_to receive(:execute) described_class.new.perform(123) end diff --git a/spec/workers/wait_for_cluster_creation_worker_spec.rb b/spec/workers/wait_for_cluster_creation_worker_spec.rb index dcd4a3b9aec..0e92b298178 100644 --- a/spec/workers/wait_for_cluster_creation_worker_spec.rb +++ b/spec/workers/wait_for_cluster_creation_worker_spec.rb @@ -2,65 +2,32 @@ require 'spec_helper' describe WaitForClusterCreationWorker do describe '#perform' do - context 'when cluster exists' do - let(:cluster) { create(:gcp_cluster) } - let(:operation) { double } + context 'when provider type is gcp' do + let(:cluster) { create(:cluster, provider_type: :gcp, provider_gcp: provider) } + let(:provider) { create(:cluster_provider_gcp, :creating) } - before do - allow(operation).to receive(:status).and_return(status) - allow(operation).to receive(:start_time).and_return(1.minute.ago) - allow(operation).to receive(:status_message).and_return('error') - allow_any_instance_of(Ci::FetchGcpOperationService).to receive(:execute).and_yield(operation) - end - - context 'when operation status is RUNNING' do - let(:status) { 'RUNNING' } - - it 'reschedules worker' do - expect(described_class).to receive(:perform_in) - - described_class.new.perform(cluster.id) - end - - context 'when operation timeout' do - before do - allow(operation).to receive(:start_time).and_return(30.minutes.ago.utc) - end - - it 'sets an error message on cluster' do - described_class.new.perform(cluster.id) + it 'provision a cluster' do + expect_any_instance_of(Clusters::Gcp::VerifyProvisionStatusService).to receive(:execute) - expect(cluster.reload).to be_errored - end - end - end - - context 'when operation status is DONE' do - let(:status) { 'DONE' } - - it 'finalizes cluster creation' do - expect_any_instance_of(Ci::FinalizeClusterCreationService).to receive(:execute) - - described_class.new.perform(cluster.id) - end + described_class.new.perform(cluster.id) end + end - context 'when operation status is others' do - let(:status) { 'others' } + context 'when provider type is user' do + let(:cluster) { create(:cluster, provider_type: :user) } - it 'sets an error message on cluster' do - described_class.new.perform(cluster.id) + it 'does not provision a cluster' do + expect_any_instance_of(Clusters::Gcp::VerifyProvisionStatusService).not_to receive(:execute) - expect(cluster.reload).to be_errored - end + described_class.new.perform(cluster.id) end end context 'when cluster does not exist' do it 'does not provision a cluster' do - expect_any_instance_of(Ci::FetchGcpOperationService).not_to receive(:execute) + expect_any_instance_of(Clusters::Gcp::VerifyProvisionStatusService).not_to receive(:execute) - described_class.new.perform(1234) + described_class.new.perform(123) end end end |