From 138d3dba298d937ddce58f2d255fb4fd68688cbc Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Fri, 3 Jul 2015 08:56:32 +0200 Subject: [PATCH] fix some specs - error messages now contain punctuation - when :save returns false, there should be errors present - errors in form representers are now API errors --- .../controllers/custom_fields_controller_spec.rb | 2 +- .../create_form_representer_spec.rb | 14 ++------------ .../update_form_representer_spec.rb | 14 ++------------ spec/models/available_project_status_spec.rb | 4 ++-- spec/models/copy_project_job_spec.rb | 2 +- spec/models/planning_element_type_color_spec.rb | 6 +++--- spec/models/project_association_spec.rb | 4 ++-- spec/models/project_type_spec.rb | 4 ++-- spec/models/reporting_spec.rb | 4 ++-- .../work_package/work_package_planning_spec.rb | 16 ++++++++-------- spec/requests/api/v3/work_packages_api_spec.rb | 8 +++++++- 11 files changed, 32 insertions(+), 46 deletions(-) diff --git a/spec/controllers/custom_fields_controller_spec.rb b/spec/controllers/custom_fields_controller_spec.rb index a329a2df6bc..9f3c7106d90 100644 --- a/spec/controllers/custom_fields_controller_spec.rb +++ b/spec/controllers/custom_fields_controller_spec.rb @@ -99,7 +99,7 @@ describe CustomFieldsController, type: :controller do end it { expect(response).to render_template 'new' } - it { expect(assigns(:custom_field).errors.messages[:name].first).to eq "can't be blank" } + it { expect(assigns(:custom_field).errors.messages[:name].first).to eq "can't be blank." } it { expect(assigns(:custom_field).translations).to be_empty } end diff --git a/spec/lib/api/v3/work_packages/create_form_representer_spec.rb b/spec/lib/api/v3/work_packages/create_form_representer_spec.rb index f8d29ab513d..7a2b0d742b8 100644 --- a/spec/lib/api/v3/work_packages/create_form_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/create_form_representer_spec.rb @@ -32,7 +32,7 @@ require 'spec_helper' describe ::API::V3::WorkPackages::CreateFormRepresenter do include API::V3::Utilities::PathHelper - let(:errors) { Hash.new } + let(:errors) { [] } let(:work_package) { FactoryGirl.build(:work_package, id: 42, @@ -46,12 +46,6 @@ describe ::API::V3::WorkPackages::CreateFormRepresenter do described_class.new(work_package, current_user: current_user, errors: errors) } - before do - # a little trick to ensure mocked properties are present on duplicated instances, too - # however, this carries the assumption that the code can handle duplication not happening - allow(errors).to receive(:dup).and_return(errors) - end - context 'generation' do subject(:generated) { representer.to_json } @@ -105,11 +99,7 @@ describe ::API::V3::WorkPackages::CreateFormRepresenter do end context 'invalid work package' do - let(:errors) { { something: [:broken] } } - - before do - allow(errors).to receive(:full_message).and_return '' - end + let(:errors) { [::API::Errors::Validation.new(:subject, 'it is broken')] } it { is_expected.not_to have_json_path('_links/commit/href') } end diff --git a/spec/lib/api/v3/work_packages/update_form_representer_spec.rb b/spec/lib/api/v3/work_packages/update_form_representer_spec.rb index 052a0b861af..5e2042b4084 100644 --- a/spec/lib/api/v3/work_packages/update_form_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/update_form_representer_spec.rb @@ -31,7 +31,7 @@ require 'spec_helper' describe ::API::V3::WorkPackages::UpdateFormRepresenter do include API::V3::Utilities::PathHelper - let(:errors) { Hash.new } + let(:errors) { [] } let(:work_package) { FactoryGirl.build(:work_package, id: 42, @@ -45,12 +45,6 @@ describe ::API::V3::WorkPackages::UpdateFormRepresenter do described_class.new(work_package, current_user: current_user, errors: errors) } - before do - # a little trick to ensure mocked properties are present on duplicated instances, too - # however, this carries the assumption that the code can handle duplication not happening - allow(errors).to receive(:dup).and_return(errors) - end - context 'generation' do subject(:generated) { representer.to_json } @@ -88,11 +82,7 @@ describe ::API::V3::WorkPackages::UpdateFormRepresenter do end context 'invalid work package' do - let(:errors) { { something: [:broken] } } - - before do - allow(errors).to receive(:full_message).and_return '' - end + let(:errors) { [::API::Errors::Validation.new(:subject, 'it is broken')] } it { is_expected.not_to have_json_path('_links/commit/href') } end diff --git a/spec/models/available_project_status_spec.rb b/spec/models/available_project_status_spec.rb index fec395598f9..c02ce5b4f84 100644 --- a/spec/models/available_project_status_spec.rb +++ b/spec/models/available_project_status_spec.rb @@ -76,7 +76,7 @@ describe AvailableProjectStatus, type: :model do expect(available_project_status).not_to be_valid expect(available_project_status.errors[:project_type]).to be_present - expect(available_project_status.errors[:project_type]).to eq(["can't be blank"]) + expect(available_project_status.errors[:project_type]).to eq(["can't be blank."]) end end @@ -88,7 +88,7 @@ describe AvailableProjectStatus, type: :model do expect(available_project_status).not_to be_valid expect(available_project_status.errors[:reported_project_status]).to be_present - expect(available_project_status.errors[:reported_project_status]).to eq(["can't be blank"]) + expect(available_project_status.errors[:reported_project_status]).to eq(["can't be blank."]) end end end diff --git a/spec/models/copy_project_job_spec.rb b/spec/models/copy_project_job_spec.rb index 116c1873a73..f8630a6e63a 100644 --- a/spec/models/copy_project_job_spec.rb +++ b/spec/models/copy_project_job_spec.rb @@ -88,7 +88,7 @@ describe CopyProjectJob, type: :model do false } # send mails let(:params) { { name: 'Copy', identifier: 'copy', type_ids: [type.id], work_package_custom_field_ids: [custom_field.id] } } - let(:expected_error_message) { "#{WorkPackage.model_name.human} '#{work_package.type.name} #: #{work_package.subject}': #{custom_field.name} #{I18n.t('errors.messages.blank')}" } + let(:expected_error_message) { "#{WorkPackage.model_name.human} '#{work_package.type.name} #: #{work_package.subject}': #{custom_field.name} #{I18n.t('errors.messages.blank')}." } before do source_project.work_package_custom_fields << custom_field diff --git a/spec/models/planning_element_type_color_spec.rb b/spec/models/planning_element_type_color_spec.rb index 1deac46bc51..ed8d4d1febb 100644 --- a/spec/models/planning_element_type_color_spec.rb +++ b/spec/models/planning_element_type_color_spec.rb @@ -70,7 +70,7 @@ describe PlanningElementTypeColor, type: :model do expect(color).not_to be_valid expect(color.errors[:name]).to be_present - expect(color.errors[:name]).to eq(["can't be blank"]) + expect(color.errors[:name]).to eq(["can't be blank."]) end it 'is invalid w/ a name longer than 255 characters' do @@ -80,7 +80,7 @@ describe PlanningElementTypeColor, type: :model do expect(color).not_to be_valid expect(color.errors[:name]).to be_present - expect(color.errors[:name]).to eq(['is too long (maximum is 255 characters)']) + expect(color.errors[:name]).to eq(['is too long (maximum is 255 characters).']) end end @@ -92,7 +92,7 @@ describe PlanningElementTypeColor, type: :model do expect(color).not_to be_valid expect(color.errors[:hexcode]).to be_present - expect(color.errors[:hexcode]).to eq(["can't be blank"]) + expect(color.errors[:hexcode]).to eq(["can't be blank."]) end it 'is invalid w/ malformed hexcodes' do diff --git a/spec/models/project_association_spec.rb b/spec/models/project_association_spec.rb index b3d9a06cd0b..87d407d7f86 100644 --- a/spec/models/project_association_spec.rb +++ b/spec/models/project_association_spec.rb @@ -102,7 +102,7 @@ describe ProjectAssociation, type: :model do expect(project_association).not_to be_valid - expect(project_association.errors[:project_a]).to eq(["can't be blank"]) + expect(project_association.errors[:project_a]).to eq(["can't be blank."]) end end @@ -113,7 +113,7 @@ describe ProjectAssociation, type: :model do expect(project_association).not_to be_valid - expect(project_association.errors[:project_b]).to eq(["can't be blank"]) + expect(project_association.errors[:project_b]).to eq(["can't be blank."]) end end end diff --git a/spec/models/project_type_spec.rb b/spec/models/project_type_spec.rb index dacffb9e177..20ac37aaf78 100644 --- a/spec/models/project_type_spec.rb +++ b/spec/models/project_type_spec.rb @@ -89,7 +89,7 @@ describe ProjectType, type: :model do expect(project_type).not_to be_valid expect(project_type.errors[:name]).to be_present - expect(project_type.errors[:name]).to eq(["can't be blank"]) + expect(project_type.errors[:name]).to eq(["can't be blank."]) end it 'is invalid w/ a name longer than 255 characters' do @@ -99,7 +99,7 @@ describe ProjectType, type: :model do expect(project_type).not_to be_valid expect(project_type.errors[:name]).to be_present - expect(project_type.errors[:name]).to eq(['is too long (maximum is 255 characters)']) + expect(project_type.errors[:name]).to eq(['is too long (maximum is 255 characters).']) end end diff --git a/spec/models/reporting_spec.rb b/spec/models/reporting_spec.rb index 8f09008ba64..a377e4e5cd7 100644 --- a/spec/models/reporting_spec.rb +++ b/spec/models/reporting_spec.rb @@ -85,7 +85,7 @@ describe Reporting, type: :model do expect(reporting).not_to be_valid expect(reporting.errors[:project]).to be_present - expect(reporting.errors[:project]).to eq(["can't be blank"]) + expect(reporting.errors[:project]).to eq(["can't be blank."]) end end @@ -98,7 +98,7 @@ describe Reporting, type: :model do expect(reporting).not_to be_valid expect(reporting.errors[:reporting_to_project]).to be_present - expect(reporting.errors[:reporting_to_project]).to eq(["can't be blank"]) + expect(reporting.errors[:reporting_to_project]).to eq(["can't be blank."]) end end end diff --git a/spec/models/work_package/work_package_planning_spec.rb b/spec/models/work_package/work_package_planning_spec.rb index f18f4319d02..366dc62a179 100644 --- a/spec/models/work_package/work_package_planning_spec.rb +++ b/spec/models/work_package/work_package_planning_spec.rb @@ -103,7 +103,7 @@ describe WorkPackage, type: :model do expect(planning_element).not_to be_valid expect(planning_element.errors[:subject]).to be_present - expect(planning_element.errors[:subject]).to eq(["can't be blank"]) + expect(planning_element.errors[:subject]).to eq(["can't be blank."]) end it 'is invalid w/ a subject longer than 255 characters' do @@ -113,7 +113,7 @@ describe WorkPackage, type: :model do expect(planning_element).not_to be_valid expect(planning_element.errors[:subject]).to be_present - expect(planning_element.errors[:subject]).to eq(['is too long (maximum is 255 characters)']) + expect(planning_element.errors[:subject]).to eq(['is too long (maximum is 255 characters).']) end end @@ -146,7 +146,7 @@ describe WorkPackage, type: :model do expect(planning_element).not_to be_valid expect(planning_element.errors[:due_date]).to be_present - expect(planning_element.errors[:due_date]).to eq(['must be greater than start date']) + expect(planning_element.errors[:due_date]).to eq(['must be greater than start date.']) end it 'is invalid if planning_element is milestone and due_date is not on start_date' do @@ -158,7 +158,7 @@ describe WorkPackage, type: :model do expect(planning_element).not_to be_valid expect(planning_element.errors[:due_date]).to be_present - expect(planning_element.errors[:due_date]).to eq(['is not on start date, although this is required for milestones']) + expect(planning_element.errors[:due_date]).to eq(['is not on start date, although this is required for milestones.']) end end @@ -170,13 +170,13 @@ describe WorkPackage, type: :model do expect(planning_element).not_to be_valid expect(planning_element.errors[:project]).to be_present - expect(planning_element.errors[:project]).to eq(["can't be blank"]) + expect(planning_element.errors[:project]).to eq(["can't be blank."]) end end describe 'parent' do - let (:de_message) { 'darf kein Meilenstein sein' } - let (:en_message) { 'cannot be a milestone' } + let (:de_message) { 'darf kein Meilenstein sein.' } + let (:en_message) { 'cannot be a milestone.' } after(:each) do # proper reset of the locale after the test I18n.locale = 'en' @@ -195,7 +195,7 @@ describe WorkPackage, type: :model do expect(planning_element).not_to be_valid expect(planning_element.errors[:parent_id]).to be_present - expect(planning_element.errors[:parent_id]).to eq([send("#{I18n.locale}_message")]) + expect(planning_element.errors[:parent_id]).to eq([send("#{locale}_message")]) end end diff --git a/spec/requests/api/v3/work_packages_api_spec.rb b/spec/requests/api/v3/work_packages_api_spec.rb index 1872fae22a0..86435bdfc1d 100644 --- a/spec/requests/api/v3/work_packages_api_spec.rb +++ b/spec/requests/api/v3/work_packages_api_spec.rb @@ -57,7 +57,13 @@ describe API::V3::WorkPackages::WorkPackagesAPI, type: :request do end it_behaves_like 'invalid activity request' do - before { allow_any_instance_of(WorkPackage).to receive(:save).and_return(false) } + before do + work_package.errors.add :base, :invalid + + # Using allow_any_instance because we don't control the WP returned to the API + allow_any_instance_of(WorkPackage).to receive(:save).and_return(false) + allow_any_instance_of(WorkPackage).to receive(:errors).and_return(work_package.errors) + end include_context 'create activity' end