From c5bca03bf0199c8bbaabcc278b3a06b30c1e8115 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Mon, 15 Oct 2018 10:24:25 +1300 Subject: Use subject in controller spec Swap out `go` method with subject which is the convention. Re-organize 'PUT update' to remove un-necessary context nesting. DRY up repeated blocks to `add_maintainer` and `sign_in` --- .../projects/clusters_controller_spec.rb | 463 +++++++++------------ 1 file changed, 202 insertions(+), 261 deletions(-) (limited to 'spec/controllers/projects') diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index 97ac11fd171..b6917479ff9 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -6,21 +6,25 @@ describe Projects::ClustersController do set(:project) { create(:project) } - describe 'GET index' do - describe 'functionality' do - let(:user) { create(:user) } + let(:user) { create(:user) } - before do - project.add_maintainer(user) - sign_in(user) - end + before do + project.add_maintainer(user) + sign_in(user) + end + describe 'GET index' do + subject do + get :index, namespace_id: project.namespace.to_param, project_id: project + end + + describe 'functionality' do context 'when project has one or more clusters' do let(:project) { create(:project) } let!(:enabled_cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } let!(:disabled_cluster) { create(:cluster, :disabled, :provided_by_gcp, :production_environment, projects: [project]) } it 'lists available clusters' do - go + subject expect(response).to have_gitlab_http_status(:ok) expect(response).to render_template(:index) @@ -30,13 +34,18 @@ describe Projects::ClustersController do context 'when page is specified' do let(:last_page) { project.clusters.page.total_pages } + subject do + get :index, namespace_id: project.namespace, project_id: project, page: last_page + end + before do allow(Clusters::Cluster).to receive(:paginates_per).and_return(1) create_list(:cluster, 2, :provided_by_gcp, :production_environment, projects: [project]) - get :index, namespace_id: project.namespace, project_id: project, page: last_page end it 'redirects to the page' do + subject + expect(response).to have_gitlab_http_status(:ok) expect(assigns(:clusters).current_page).to eq(last_page) end @@ -47,7 +56,7 @@ describe Projects::ClustersController do let(:project) { create(:project) } it 'returns an empty state page' do - go + subject expect(response).to have_gitlab_http_status(:ok) expect(response).to render_template(:index, partial: :empty_state) @@ -59,30 +68,23 @@ describe Projects::ClustersController do describe 'security' do let(:cluster) { create(:cluster, :provided_by_gcp, projects: [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(:maintainer).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 :index, namespace_id: project.namespace.to_param, project_id: project + it { expect { subject }.to be_allowed_for(:admin) } + it { expect { subject }.to be_allowed_for(:owner).of(project) } + it { expect { subject }.to be_allowed_for(:maintainer).of(project) } + it { expect { subject }.to be_denied_for(:developer).of(project) } + it { expect { subject }.to be_denied_for(:reporter).of(project) } + it { expect { subject }.to be_denied_for(:guest).of(project) } + it { expect { subject }.to be_denied_for(:user) } + it { expect { subject }.to be_denied_for(:external) } end end describe 'GET new' do - describe 'functionality for new cluster' do - let(:user) { create(:user) } - - before do - project.add_maintainer(user) - sign_in(user) - end + subject do + get :new, namespace_id: project.namespace, project_id: project + end + describe 'functionality for new cluster' do context 'when omniauth has been configured' do let(:key) { 'secret-key' } let(:session_key_for_redirect_uri) do @@ -94,7 +96,7 @@ describe Projects::ClustersController do end it 'has authorize_url' do - go + subject expect(assigns(:authorize_url)).to include(key) expect(session[session_key_for_redirect_uri]).to eq(new_project_cluster_path(project)) @@ -107,7 +109,7 @@ describe Projects::ClustersController do end it 'does not have authorize_url' do - go + subject expect(assigns(:authorize_url)).to be_nil end @@ -119,7 +121,7 @@ describe Projects::ClustersController do end it 'has new object' do - go + subject expect(assigns(:gcp_cluster)).to be_an_instance_of(Clusters::Cluster) end @@ -139,33 +141,22 @@ describe Projects::ClustersController do end describe 'functionality for existing cluster' do - let(:user) { create(:user) } - - before do - project.add_maintainer(user) - sign_in(user) - end - it 'has new object' do - go + subject expect(assigns(:user_cluster)).to be_an_instance_of(Clusters::Cluster) 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(:maintainer).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 + it { expect { subject }.to be_allowed_for(:admin) } + it { expect { subject }.to be_allowed_for(:owner).of(project) } + it { expect { subject }.to be_allowed_for(:maintainer).of(project) } + it { expect { subject }.to be_denied_for(:developer).of(project) } + it { expect { subject }.to be_denied_for(:reporter).of(project) } + it { expect { subject }.to be_denied_for(:guest).of(project) } + it { expect { subject }.to be_denied_for(:user) } + it { expect { subject }.to be_denied_for(:external) } end end @@ -183,14 +174,11 @@ describe Projects::ClustersController do } end - describe 'functionality' do - let(:user) { create(:user) } - - before do - project.add_maintainer(user) - sign_in(user) - end + subject do + post :create_gcp, params.merge(namespace_id: project.namespace, project_id: project) + end + describe 'functionality' do context 'when access token is valid' do before do stub_google_api_validate_token @@ -198,7 +186,7 @@ describe Projects::ClustersController do it 'creates a new cluster' do expect(ClusterProvisionWorker).to receive(:perform_async) - expect { go }.to change { Clusters::Cluster.count } + expect { subject }.to change { Clusters::Cluster.count } .and change { Clusters::Providers::Gcp.count } expect(response).to redirect_to(project_cluster_path(project, project.clusters.first)) expect(project.clusters.first).to be_gcp @@ -211,7 +199,7 @@ describe Projects::ClustersController do it 'creates a new cluster with legacy_abac_disabled' do expect(ClusterProvisionWorker).to receive(:perform_async) - expect { go }.to change { Clusters::Cluster.count } + expect { subject }.to change { Clusters::Cluster.count } .and change { Clusters::Providers::Gcp.count } expect(project.clusters.first.provider_gcp).not_to be_legacy_abac end @@ -248,18 +236,14 @@ describe Projects::ClustersController do allow(WaitForClusterCreationWorker).to receive(:perform_in).and_return(nil) end - 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(:maintainer).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_gcp, params.merge(namespace_id: project.namespace, project_id: project) + it { expect { subject }.to be_allowed_for(:admin) } + it { expect { subject }.to be_allowed_for(:owner).of(project) } + it { expect { subject }.to be_allowed_for(:maintainer).of(project) } + it { expect { subject }.to be_denied_for(:developer).of(project) } + it { expect { subject }.to be_denied_for(:reporter).of(project) } + it { expect { subject }.to be_denied_for(:guest).of(project) } + it { expect { subject }.to be_denied_for(:user) } + it { expect { subject }.to be_denied_for(:external) } end end @@ -277,19 +261,16 @@ describe Projects::ClustersController do } end - describe 'functionality' do - let(:user) { create(:user) } - - before do - project.add_maintainer(user) - sign_in(user) - end + subject do + post :create_user, params.merge(namespace_id: project.namespace, project_id: project) + end + describe 'functionality' do context 'when creates a cluster' do it 'creates a new cluster' do expect(ClusterProvisionWorker).to receive(:perform_async) - expect { go }.to change { Clusters::Cluster.count } + expect { subject }.to change { Clusters::Cluster.count } .and change { Clusters::Platforms::Kubernetes.count } expect(response).to redirect_to(project_cluster_path(project, project.clusters.first)) @@ -317,7 +298,7 @@ describe Projects::ClustersController do it 'creates a new cluster' do expect(ClusterProvisionWorker).to receive(:perform_async) - expect { go }.to change { Clusters::Cluster.count } + expect { subject }.to change { Clusters::Cluster.count } .and change { Clusters::Platforms::Kubernetes.count } expect(response).to redirect_to(project_cluster_path(project, project.clusters.first)) @@ -330,34 +311,30 @@ describe Projects::ClustersController do 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(:maintainer).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_user, params.merge(namespace_id: project.namespace, project_id: project) + it { expect { subject }.to be_allowed_for(:admin) } + it { expect { subject }.to be_allowed_for(:owner).of(project) } + it { expect { subject }.to be_allowed_for(:maintainer).of(project) } + it { expect { subject }.to be_denied_for(:developer).of(project) } + it { expect { subject }.to be_denied_for(:reporter).of(project) } + it { expect { subject }.to be_denied_for(:guest).of(project) } + it { expect { subject }.to be_denied_for(:user) } + it { expect { subject }.to be_denied_for(:external) } end end describe 'GET status' do let(:cluster) { create(:cluster, :providing_by_gcp, projects: [project]) } - describe 'functionality' do - let(:user) { create(:user) } - - before do - project.add_maintainer(user) - sign_in(user) - end + subject do + get :status, namespace_id: project.namespace, + project_id: project, + id: cluster, + format: :json + end + describe 'functionality' do it "responds with matching schema" do - go + subject expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('cluster_status') @@ -366,42 +343,34 @@ describe Projects::ClustersController do it 'invokes schedule_status_update on each application' do expect_any_instance_of(Clusters::Applications::Ingress).to receive(:schedule_status_update) - go + subject 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(:maintainer).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 + it { expect { subject }.to be_allowed_for(:admin) } + it { expect { subject }.to be_allowed_for(:owner).of(project) } + it { expect { subject }.to be_allowed_for(:maintainer).of(project) } + it { expect { subject }.to be_denied_for(:developer).of(project) } + it { expect { subject }.to be_denied_for(:reporter).of(project) } + it { expect { subject }.to be_denied_for(:guest).of(project) } + it { expect { subject }.to be_denied_for(:user) } + it { expect { subject }.to be_denied_for(:external) } end end describe 'GET show' do let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } - describe 'functionality' do - let(:user) { create(:user) } - - before do - project.add_maintainer(user) - sign_in(user) - end + subject do + get :show, namespace_id: project.namespace, + project_id: project, + id: cluster + end + describe 'functionality' do it "renders view" do - go + subject expect(response).to have_gitlab_http_status(:ok) expect(assigns(:cluster)).to eq(cluster) @@ -409,85 +378,108 @@ describe Projects::ClustersController do 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(:maintainer).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 :show, namespace_id: project.namespace, - project_id: project, - id: cluster + it { expect { subject }.to be_allowed_for(:admin) } + it { expect { subject }.to be_allowed_for(:owner).of(project) } + it { expect { subject }.to be_allowed_for(:maintainer).of(project) } + it { expect { subject }.to be_denied_for(:developer).of(project) } + it { expect { subject }.to be_denied_for(:reporter).of(project) } + it { expect { subject }.to be_denied_for(:guest).of(project) } + it { expect { subject }.to be_denied_for(:user) } + it { expect { subject }.to be_denied_for(:external) } end end describe 'PUT update' do - context 'when cluster is provided by GCP' do - let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } - let(:user) { create(:user) } - - before do - project.add_maintainer(user) - sign_in(user) - end + let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } - context 'when changing parameters' do - let(:params) do - { - cluster: { - enabled: false, - name: 'my-new-cluster-name', - platform_kubernetes_attributes: { - namespace: 'my-namespace' - } - } + let(:params) do + { + cluster: { + enabled: false, + name: 'my-new-cluster-name', + platform_kubernetes_attributes: { + namespace: 'my-namespace' } - end + } + } + end - it "updates and redirects back to show page" do - go + subject do + put :update, params.merge(namespace_id: project.namespace, + project_id: project, + id: cluster) + end - cluster.reload - expect(response).to redirect_to(project_cluster_path(project, cluster)) - expect(flash[:notice]).to eq('Kubernetes cluster was successfully updated.') - expect(cluster.enabled).to be_falsey - end + context 'when cluster is provided by GCP' do + it "updates and redirects back to show page" do + subject - it "does not change cluster name" do - go + cluster.reload + expect(response).to redirect_to(project_cluster_path(project, cluster)) + expect(flash[:notice]).to eq('Kubernetes cluster was successfully updated.') + expect(cluster.enabled).to be_falsey + end - expect(cluster.name).to eq('test-cluster') - end + it "does not change cluster name" do + subject - context 'when cluster is being created' do - let(:cluster) { create(:cluster, :providing_by_gcp, projects: [project]) } + cluster.reload + expect(cluster.name).to eq('test-cluster') + end - it "rejects changes" do - go + context 'when cluster is being created' do + let(:cluster) { create(:cluster, :providing_by_gcp, projects: [project]) } - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:show) - expect(cluster.enabled).to be_truthy - end + it "rejects changes" do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:show) + expect(cluster.enabled).to be_truthy end end end context 'when cluster is provided by user' do let(:cluster) { create(:cluster, :provided_by_user, projects: [project]) } - let(:user) { create(:user) } - before do - project.add_maintainer(user) - sign_in(user) + let(:params) do + { + cluster: { + enabled: false, + name: 'my-new-cluster-name', + platform_kubernetes_attributes: { + namespace: 'my-namespace' + } + } + } + end + + subject do + put :update, params.merge(namespace_id: project.namespace, + project_id: project, + id: cluster) + end + + it "updates and redirects back to show page" do + subject + + cluster.reload + expect(response).to redirect_to(project_cluster_path(project, cluster)) + expect(flash[:notice]).to eq('Kubernetes cluster was successfully updated.') + expect(cluster.enabled).to be_falsey + expect(cluster.name).to eq('my-new-cluster-name') + expect(cluster.platform_kubernetes.namespace).to eq('my-namespace') end context 'when format is json' do + subject do + put :update, params.merge(namespace_id: project.namespace, + project_id: project, + id: cluster, + format: :json) + end + context 'when changing parameters' do context 'when valid parameters are used' do let(:params) do @@ -503,7 +495,7 @@ describe Projects::ClustersController do end it "updates and redirects back to show page" do - go_json + subject cluster.reload expect(response).to have_http_status(:no_content) @@ -526,88 +518,43 @@ describe Projects::ClustersController do end it "rejects changes" do - go_json + subject expect(response).to have_http_status(:bad_request) end end end end - - context 'when format is html' do - context 'when update enabled' do - let(:params) do - { - cluster: { - enabled: false, - name: 'my-new-cluster-name', - platform_kubernetes_attributes: { - namespace: 'my-namespace' - } - } - } - end - - it "updates and redirects back to show page" do - go - - cluster.reload - expect(response).to redirect_to(project_cluster_path(project, cluster)) - expect(flash[:notice]).to eq('Kubernetes cluster was successfully updated.') - expect(cluster.enabled).to be_falsey - expect(cluster.name).to eq('my-new-cluster-name') - expect(cluster.platform_kubernetes.namespace).to eq('my-namespace') - end - end - end end describe 'security' do set(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } - let(:params) do - { cluster: { enabled: false } } - end - - 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(:maintainer).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 - put :update, params.merge(namespace_id: project.namespace, - project_id: project, - id: cluster) - end - - def go_json - put :update, params.merge(namespace_id: project.namespace, - project_id: project, - id: cluster, - format: :json) + it { expect { subject }.to be_allowed_for(:admin) } + it { expect { subject }.to be_allowed_for(:owner).of(project) } + it { expect { subject }.to be_allowed_for(:maintainer).of(project) } + it { expect { subject }.to be_denied_for(:developer).of(project) } + it { expect { subject }.to be_denied_for(:reporter).of(project) } + it { expect { subject }.to be_denied_for(:guest).of(project) } + it { expect { subject }.to be_denied_for(:user) } + it { expect { subject }.to be_denied_for(:external) } end end describe 'DELETE destroy' do - describe 'functionality' do - let(:user) { create(:user) } + let!(:cluster) { create(:cluster, :provided_by_gcp, :production_environment, projects: [project]) } - before do - project.add_maintainer(user) - sign_in(user) - end + subject do + delete :destroy, namespace_id: project.namespace, + project_id: project, + id: cluster + end + describe 'functionality' do context 'when cluster is provided by GCP' do context 'when cluster is created' do - let!(:cluster) { create(:cluster, :provided_by_gcp, :production_environment, projects: [project]) } - it "destroys and redirects back to clusters list" do - expect { go } + expect { subject } .to change { Clusters::Cluster.count }.by(-1) .and change { Clusters::Platforms::Kubernetes.count }.by(-1) .and change { Clusters::Providers::Gcp.count }.by(-1) @@ -621,7 +568,7 @@ describe Projects::ClustersController do let!(:cluster) { create(:cluster, :providing_by_gcp, :production_environment, projects: [project]) } it "destroys and redirects back to clusters list" do - expect { go } + expect { subject } .to change { Clusters::Cluster.count }.by(-1) .and change { Clusters::Providers::Gcp.count }.by(-1) @@ -635,7 +582,7 @@ describe Projects::ClustersController do let!(:cluster) { create(:cluster, :provided_by_user, :production_environment, projects: [project]) } it "destroys and redirects back to clusters list" do - expect { go } + expect { subject } .to change { Clusters::Cluster.count }.by(-1) .and change { Clusters::Platforms::Kubernetes.count }.by(-1) .and change { Clusters::Providers::Gcp.count }.by(0) @@ -649,20 +596,14 @@ describe Projects::ClustersController do describe 'security' do set(:cluster) { create(:cluster, :provided_by_gcp, :production_environment, projects: [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(:maintainer).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 - delete :destroy, namespace_id: project.namespace, - project_id: project, - id: cluster + it { expect { subject }.to be_allowed_for(:admin) } + it { expect { subject }.to be_allowed_for(:owner).of(project) } + it { expect { subject }.to be_allowed_for(:maintainer).of(project) } + it { expect { subject }.to be_denied_for(:developer).of(project) } + it { expect { subject }.to be_denied_for(:reporter).of(project) } + it { expect { subject }.to be_denied_for(:guest).of(project) } + it { expect { subject }.to be_denied_for(:user) } + it { expect { subject }.to be_denied_for(:external) } end end end -- cgit v1.2.1