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

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
 

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

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

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

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

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

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

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
 

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

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

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