Commit 82bc8ca48aacd98fc56508eb8da1d7eab120ca78
Committed by
Rafael Manzo
1 parent
987ca379
Exists in
colab
and in
4 other branches
Update MetricConfiguration controllers' tests
- Fix usage of MetricConfiguration models converted directly to hashes as parameters. It didn't reflect how they are actually sent and omitted some necessary parameters like metric_collector_name and metric_name/metric_code, which are now passed. - Fix KalibroConfigurations not being searched by the controller methods. We must be sure they exist, and fail with a 404 if they don't instead of failing only in one of the ownership testing methods. - Update the expectations for cache cleaning to include both Hotspot and Tree MetricConfigurations Signed off by: Daniel Miranda <danielkza2@gmail.com>
Showing
3 changed files
with
48 additions
and
23 deletions
Show diff stats
spec/controllers/compound_metric_configurations_controller_spec.rb
| ... | ... | @@ -11,6 +11,7 @@ describe CompoundMetricConfigurationsController, :type => :controller do |
| 11 | 11 | context 'when the current user owns the kalibro configuration' do |
| 12 | 12 | let!(:metric_configuration) { FactoryGirl.build(:metric_configuration) } |
| 13 | 13 | before :each do |
| 14 | + KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns kalibro_configuration | |
| 14 | 15 | subject.expects(:kalibro_configuration_owner?).returns true |
| 15 | 16 | MetricConfiguration.expects(:metric_configurations_of).with(kalibro_configuration.id).returns([metric_configuration]) |
| 16 | 17 | get :new, kalibro_configuration_id: kalibro_configuration.id |
| ... | ... | @@ -31,24 +32,24 @@ describe CompoundMetricConfigurationsController, :type => :controller do |
| 31 | 32 | end |
| 32 | 33 | |
| 33 | 34 | describe 'create' do |
| 34 | - let!(:metric_configuration_params) { FactoryGirl.build(:metric_configuration).to_hash } | |
| 35 | - let!(:metric_params) { FactoryGirl.build(:metric).to_hash } | |
| 36 | - let(:compound_metric_configuration) { FactoryGirl.build(:compound_metric_configuration) } | |
| 35 | + let(:compound_metric_configuration) { FactoryGirl.build(:compound_metric_configuration, reading_group_id: 1) } | |
| 36 | + let!(:metric_configuration_params) { compound_metric_configuration.to_hash } | |
| 37 | 37 | |
| 38 | 38 | before do |
| 39 | 39 | sign_in FactoryGirl.create(:user) |
| 40 | - metric_configuration_params["metric"] = metric_params | |
| 41 | 40 | end |
| 42 | 41 | |
| 43 | 42 | context 'when the current user owns the reading group' do |
| 44 | 43 | before :each do |
| 44 | + KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns kalibro_configuration | |
| 45 | 45 | subject.expects(:kalibro_configuration_owner?).returns true |
| 46 | - Rails.cache.expects(:delete).with("#{kalibro_configuration.id}_tree_metric_configurations") | |
| 47 | 46 | end |
| 48 | 47 | |
| 49 | 48 | context 'with valid fields' do |
| 50 | 49 | before :each do |
| 51 | 50 | MetricConfiguration.any_instance.expects(:save).returns(true) |
| 51 | + Rails.cache.expects(:delete).with("#{kalibro_configuration.id}_tree_metric_configurations") | |
| 52 | + Rails.cache.expects(:delete).with("#{kalibro_configuration.id}_hotspot_metric_configurations") | |
| 52 | 53 | |
| 53 | 54 | post :create, kalibro_configuration_id: kalibro_configuration.id, metric_configuration: metric_configuration_params |
| 54 | 55 | end |
| ... | ... | @@ -59,7 +60,6 @@ describe CompoundMetricConfigurationsController, :type => :controller do |
| 59 | 60 | context 'with invalid fields' do |
| 60 | 61 | before :each do |
| 61 | 62 | MetricConfiguration.any_instance.expects(:save).returns(false) |
| 62 | - MetricConfiguration.expects(:metric_configurations_of).with(kalibro_configuration.id).returns([compound_metric_configuration]) | |
| 63 | 63 | post :create, kalibro_configuration_id: kalibro_configuration.id, metric_configuration: metric_configuration_params |
| 64 | 64 | end |
| 65 | 65 | |
| ... | ... | @@ -74,6 +74,7 @@ describe CompoundMetricConfigurationsController, :type => :controller do |
| 74 | 74 | let(:kalibro_range) { FactoryGirl.build(:kalibro_range) } |
| 75 | 75 | |
| 76 | 76 | before :each do |
| 77 | + KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns kalibro_configuration | |
| 77 | 78 | ReadingGroup.expects(:find).with(compound_metric_configuration.reading_group_id).returns(reading_group) |
| 78 | 79 | MetricConfiguration.expects(:find).with(compound_metric_configuration.id).returns(compound_metric_configuration) |
| 79 | 80 | compound_metric_configuration.expects(:kalibro_ranges).returns([kalibro_range]) |
| ... | ... | @@ -94,6 +95,7 @@ describe CompoundMetricConfigurationsController, :type => :controller do |
| 94 | 95 | |
| 95 | 96 | context 'when the user owns the compound metric configuration' do |
| 96 | 97 | before :each do |
| 98 | + KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns kalibro_configuration | |
| 97 | 99 | subject.expects(:metric_configuration_owner?).returns(true) |
| 98 | 100 | MetricConfiguration.expects(:find).with(compound_metric_configuration.id).returns(compound_metric_configuration) |
| 99 | 101 | MetricConfiguration.expects(:metric_configurations_of).with(kalibro_configuration.id).returns([compound_metric_configuration]) |
| ... | ... | @@ -125,7 +127,14 @@ describe CompoundMetricConfigurationsController, :type => :controller do |
| 125 | 127 | |
| 126 | 128 | describe 'update' do |
| 127 | 129 | let(:compound_metric_configuration) { FactoryGirl.build(:compound_metric_configuration_with_id) } |
| 128 | - let(:metric_configuration_params) { Hash[FactoryGirl.attributes_for(:compound_metric_configuration_with_id).map { |k,v| [k.to_s, v.to_s] }] } #FIXME: Mocha is creating the expectations with strings, but FactoryGirl returns everything with sybols and integers | |
| 130 | + # Exclude the parameters that come in the URL (as the ones in the body will always be ignored) | |
| 131 | + let(:metric_configuration_params) { compound_metric_configuration.to_hash.except('id', 'kalibro_configuration_id', 'type') } | |
| 132 | + # Exclude the reading group id since it is set beforehand and not in the update params | |
| 133 | + let(:update_params) do | |
| 134 | + params = metric_configuration_params.except('reading_group_id') | |
| 135 | + params['metric'].except!('type') | |
| 136 | + params | |
| 137 | + end | |
| 129 | 138 | |
| 130 | 139 | context 'when the user is logged in' do |
| 131 | 140 | before do |
| ... | ... | @@ -134,14 +143,16 @@ describe CompoundMetricConfigurationsController, :type => :controller do |
| 134 | 143 | |
| 135 | 144 | context 'when user owns the metric configuration' do |
| 136 | 145 | before :each do |
| 146 | + KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns kalibro_configuration | |
| 147 | + MetricConfiguration.expects(:find).with(compound_metric_configuration.id).returns(compound_metric_configuration) | |
| 137 | 148 | subject.expects(:metric_configuration_owner?).returns true |
| 138 | - Rails.cache.expects(:delete).with("#{kalibro_configuration.id}_tree_metric_configurations") | |
| 139 | 149 | end |
| 140 | 150 | |
| 141 | 151 | context 'with valid fields' do |
| 142 | 152 | before :each do |
| 143 | - MetricConfiguration.expects(:find).with(compound_metric_configuration.id).returns(compound_metric_configuration) | |
| 144 | - MetricConfiguration.any_instance.expects(:update).with(metric_configuration_params).returns(true) | |
| 153 | + MetricConfiguration.any_instance.expects(:update).with(update_params).returns(true) | |
| 154 | + Rails.cache.expects(:delete).with("#{kalibro_configuration.id}_tree_metric_configurations") | |
| 155 | + Rails.cache.expects(:delete).with("#{kalibro_configuration.id}_hotspot_metric_configurations") | |
| 145 | 156 | |
| 146 | 157 | post :update, kalibro_configuration_id: compound_metric_configuration.kalibro_configuration_id, id: compound_metric_configuration.id, metric_configuration: metric_configuration_params |
| 147 | 158 | end |
| ... | ... | @@ -152,9 +163,7 @@ describe CompoundMetricConfigurationsController, :type => :controller do |
| 152 | 163 | |
| 153 | 164 | context 'with an invalid field' do |
| 154 | 165 | before :each do |
| 155 | - MetricConfiguration.expects(:find).with(compound_metric_configuration.id).returns(compound_metric_configuration) | |
| 156 | - MetricConfiguration.expects(:metric_configurations_of).with(kalibro_configuration.id).returns([compound_metric_configuration]) | |
| 157 | - MetricConfiguration.any_instance.expects(:update).with(metric_configuration_params).returns(false) | |
| 166 | + MetricConfiguration.any_instance.expects(:update).with(update_params).returns(false) | |
| 158 | 167 | |
| 159 | 168 | post :update, kalibro_configuration_id: compound_metric_configuration.kalibro_configuration_id, id: compound_metric_configuration.id, metric_configuration: metric_configuration_params |
| 160 | 169 | end | ... | ... |
spec/controllers/hotspot_metric_configuration_controller_spec.rb
| ... | ... | @@ -2,6 +2,7 @@ require 'rails_helper' |
| 2 | 2 | |
| 3 | 3 | describe HotspotMetricConfigurationsController, :type => :controller do |
| 4 | 4 | let(:kalibro_configuration) { FactoryGirl.build(:kalibro_configuration, :with_id) } |
| 5 | + | |
| 5 | 6 | describe 'create' do |
| 6 | 7 | let!(:metric_configuration) { FactoryGirl.build(:hotspot_metric_configuration) } |
| 7 | 8 | let(:metric_configuration_params) { metric_configuration.to_hash } |
| ... | ... | @@ -13,6 +14,7 @@ describe HotspotMetricConfigurationsController, :type => :controller do |
| 13 | 14 | |
| 14 | 15 | context 'when the current user owns the metric configuration' do |
| 15 | 16 | before :each do |
| 17 | + KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns kalibro_configuration | |
| 16 | 18 | subject.expects(:kalibro_configuration_owner?).returns true |
| 17 | 19 | end |
| 18 | 20 | ... | ... |
spec/controllers/metric_configurations_controller_spec.rb
| ... | ... | @@ -2,6 +2,7 @@ require 'rails_helper' |
| 2 | 2 | |
| 3 | 3 | describe MetricConfigurationsController, :type => :controller do |
| 4 | 4 | let(:kalibro_configuration) { FactoryGirl.build(:kalibro_configuration, :with_id) } |
| 5 | + | |
| 5 | 6 | describe 'choose_metric' do |
| 6 | 7 | let(:metric_collector) { FactoryGirl.build(:metric_collector) } |
| 7 | 8 | before :each do |
| ... | ... | @@ -10,9 +11,9 @@ describe MetricConfigurationsController, :type => :controller do |
| 10 | 11 | |
| 11 | 12 | context 'when adding new metrics' do |
| 12 | 13 | before :each do |
| 14 | + KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns kalibro_configuration | |
| 13 | 15 | subject.expects(:kalibro_configuration_owner?).returns true |
| 14 | 16 | KalibroClient::Entities::Processor::MetricCollectorDetails.expects(:all_names).returns([metric_collector]) |
| 15 | - KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns(kalibro_configuration) | |
| 16 | 17 | get :choose_metric, kalibro_configuration_id: kalibro_configuration.id |
| 17 | 18 | end |
| 18 | 19 | |
| ... | ... | @@ -33,11 +34,12 @@ describe MetricConfigurationsController, :type => :controller do |
| 33 | 34 | |
| 34 | 35 | context 'when the current user owns the kalibro configuration' do |
| 35 | 36 | before :each do |
| 37 | + KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns kalibro_configuration | |
| 36 | 38 | ReadingGroup.expects(:public_or_owned_by_user).with(user).returns(reading_groups) |
| 37 | 39 | subject.expects(:kalibro_configuration_owner?).returns true |
| 38 | 40 | KalibroClient::Entities::Processor::MetricCollectorDetails.expects(:find_by_name).with(metric_collector.name).returns(metric_collector) |
| 39 | - metric_collector.expects(:find_metric_by_code).with(native_metric.code).returns(native_metric) | |
| 40 | - post :new, kalibro_configuration_id: kalibro_configuration.id, metric_code: native_metric.code, metric_collector_name: metric_collector.name | |
| 41 | + metric_collector.expects(:find_metric_by_name).with(native_metric.name).returns(native_metric) | |
| 42 | + get :new, kalibro_configuration_id: kalibro_configuration.id, metric_name: native_metric.name, metric_collector_name: metric_collector.name | |
| 41 | 43 | end |
| 42 | 44 | |
| 43 | 45 | it { is_expected.to respond_with(:success) } |
| ... | ... | @@ -65,6 +67,7 @@ describe MetricConfigurationsController, :type => :controller do |
| 65 | 67 | |
| 66 | 68 | context 'when the current user owns the metric configuration' do |
| 67 | 69 | before :each do |
| 70 | + KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns kalibro_configuration | |
| 68 | 71 | subject.expects(:kalibro_configuration_owner?).returns true |
| 69 | 72 | end |
| 70 | 73 | |
| ... | ... | @@ -100,6 +103,7 @@ describe MetricConfigurationsController, :type => :controller do |
| 100 | 103 | let(:kalibro_range) { FactoryGirl.build(:kalibro_range) } |
| 101 | 104 | |
| 102 | 105 | before :each do |
| 106 | + KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns kalibro_configuration | |
| 103 | 107 | ReadingGroup.expects(:find).with(metric_configuration.reading_group_id).returns(reading_group) |
| 104 | 108 | MetricConfiguration.expects(:find).with(metric_configuration.id).returns(metric_configuration) |
| 105 | 109 | metric_configuration.expects(:kalibro_ranges).returns([kalibro_range]) |
| ... | ... | @@ -122,6 +126,7 @@ describe MetricConfigurationsController, :type => :controller do |
| 122 | 126 | |
| 123 | 127 | context 'when the user owns the metric configuration' do |
| 124 | 128 | before :each do |
| 129 | + KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns kalibro_configuration | |
| 125 | 130 | ReadingGroup.expects(:public_or_owned_by_user).with(user).returns(reading_groups) |
| 126 | 131 | subject.expects(:metric_configuration_owner?).returns(true) |
| 127 | 132 | MetricConfiguration.expects(:find).with(metric_configuration.id).returns(metric_configuration) |
| ... | ... | @@ -137,7 +142,6 @@ describe MetricConfigurationsController, :type => :controller do |
| 137 | 142 | end |
| 138 | 143 | |
| 139 | 144 | it { is_expected.to redirect_to(kalibro_configurations_path(id: metric_configuration.kalibro_configuration_id)) } |
| 140 | - it { is_expected.to respond_with(:redirect) } | |
| 141 | 145 | it { is_expected.to set_flash[:notice].to("You're not allowed to do this operation") } |
| 142 | 146 | end |
| 143 | 147 | end |
| ... | ... | @@ -153,7 +157,8 @@ describe MetricConfigurationsController, :type => :controller do |
| 153 | 157 | |
| 154 | 158 | describe 'update' do |
| 155 | 159 | let(:metric_configuration) { FactoryGirl.build(:metric_configuration_with_id) } |
| 156 | - let(:metric_configuration_params) { metric_configuration.to_hash } | |
| 160 | + let(:metric_configuration_params) { metric_configuration.to_hash.except('id', 'metric') } | |
| 161 | + let(:update_params) { metric_configuration_params.except('kalibro_configuration_id', 'reading_group_id') } | |
| 157 | 162 | |
| 158 | 163 | context 'when the user is logged in' do |
| 159 | 164 | before do |
| ... | ... | @@ -162,15 +167,20 @@ describe MetricConfigurationsController, :type => :controller do |
| 162 | 167 | |
| 163 | 168 | context 'when user owns the metric configuration' do |
| 164 | 169 | before :each do |
| 170 | + KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns kalibro_configuration | |
| 165 | 171 | subject.expects(:metric_configuration_owner?).returns true |
| 166 | 172 | end |
| 167 | 173 | |
| 168 | 174 | context 'with valid fields' do |
| 169 | 175 | before :each do |
| 170 | 176 | MetricConfiguration.expects(:find).with(metric_configuration.id).returns(metric_configuration) |
| 171 | - MetricConfiguration.any_instance.expects(:update).with(metric_configuration_params).returns(true) | |
| 177 | + MetricConfiguration.any_instance.expects(:update).with(update_params).returns(true) | |
| 172 | 178 | |
| 173 | - post :update, kalibro_configuration_id: metric_configuration.kalibro_configuration_id, id: metric_configuration.id, metric_configuration: metric_configuration_params | |
| 179 | + post :update, | |
| 180 | + kalibro_configuration_id: metric_configuration.kalibro_configuration_id, id: metric_configuration.id, | |
| 181 | + metric_configuration: metric_configuration_params, | |
| 182 | + metric_collector_name: metric_configuration.metric.metric_collector_name, | |
| 183 | + metric_code: metric_configuration.metric.code | |
| 174 | 184 | end |
| 175 | 185 | |
| 176 | 186 | it { is_expected.to redirect_to(kalibro_configuration_path(id: metric_configuration.kalibro_configuration_id)) } |
| ... | ... | @@ -180,9 +190,13 @@ describe MetricConfigurationsController, :type => :controller do |
| 180 | 190 | context 'with an invalid field' do |
| 181 | 191 | before :each do |
| 182 | 192 | MetricConfiguration.expects(:find).with(metric_configuration.id).returns(metric_configuration) |
| 183 | - MetricConfiguration.any_instance.expects(:update).with(metric_configuration_params).returns(false) | |
| 193 | + MetricConfiguration.any_instance.expects(:update).with(update_params).returns(false) | |
| 184 | 194 | |
| 185 | - post :update, kalibro_configuration_id: metric_configuration.kalibro_configuration_id, id: metric_configuration.id, metric_configuration: metric_configuration_params | |
| 195 | + post :update, | |
| 196 | + kalibro_configuration_id: metric_configuration.kalibro_configuration_id, id: metric_configuration.id, | |
| 197 | + metric_configuration: metric_configuration_params, | |
| 198 | + metric_collector_name: metric_configuration.metric.metric_collector_name, | |
| 199 | + metric_code: metric_configuration.metric.code | |
| 186 | 200 | end |
| 187 | 201 | |
| 188 | 202 | it { is_expected.to render_template(:edit) } |
| ... | ... | @@ -199,7 +213,6 @@ describe MetricConfigurationsController, :type => :controller do |
| 199 | 213 | end |
| 200 | 214 | end |
| 201 | 215 | |
| 202 | - | |
| 203 | 216 | describe 'destroy' do |
| 204 | 217 | let(:metric_configuration) { FactoryGirl.build(:metric_configuration_with_id) } |
| 205 | 218 | |
| ... | ... | @@ -210,6 +223,7 @@ describe MetricConfigurationsController, :type => :controller do |
| 210 | 223 | |
| 211 | 224 | context 'when the user owns the configuration' do |
| 212 | 225 | before :each do |
| 226 | + KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns kalibro_configuration | |
| 213 | 227 | subject.expects(:metric_configuration_owner?).returns true |
| 214 | 228 | metric_configuration.expects(:destroy) |
| 215 | 229 | MetricConfiguration.expects(:find).with(metric_configuration.id).returns(metric_configuration) | ... | ... |