Adjustments based on review feedback

- Returning negative numbers from ee version of exceeded_size to match
  logic from non-ee version of method.
- Clarifications to test descriptions.
- Add more test cases.

ee/lib/ee/gitlab/repository_size_checker.rb

@@ -19,7 +19,7 @@ module EE
       def exceeded_size(change_size = 0)
         exceeded_size = super
         exceeded_size -= remaining_additional_purchased_storage if additional_repo_storage_available?
-        [exceeded_size, 0].max
+        exceeded_size
       end
 
       private

ee/spec/lib/ee/gitlab/repository_size_checker_spec.rb

@@ -140,49 +140,65 @@ RSpec.describe Gitlab::RepositorySizeChecker do
         let(:total_repository_size_excess) { 10 }
         let(:additional_purchased_storage) { 10 }
 
-        context 'when current size + total repository size excess are below or equal to the project\'s limit (no need for additional purchase storage)' do
-          let(:current_size) { 50 }
+        context 'when no change size provided' do
+          context 'when current size + total repository size excess is below the limit (additional purchase storage not used)' do
+            let(:current_size) { limit - 1 }
 
-          it 'returns zero' do
-            expect(subject.exceeded_size).to eq(0)
+            it 'returns a negative number' do
+              expect(subject.exceeded_size).to eq(-1.megabyte)
+            end
           end
-        end
 
-        context 'when there is remaining storage (current size + excess for other projects need to use additional purchased storage but not all of it)' do
-          let(:current_size) { 51 }
+          context 'when current size + total repository size excess is equal to the limit (additional purchase storage not used)' do
+            let(:current_size) { limit }
 
-          it 'returns 0' do
-            expect(subject.exceeded_size).to eq(0)
+            it 'returns zero' do
+              expect(subject.exceeded_size).to eq(0)
+            end
           end
-        end
 
-        context 'when there storage is exceeded (current size + excess for other projects exceed additional purchased storage' do
-          let(:total_repository_size_excess) { 15 }
-          let(:current_size) { 61 }
+          context 'when there is remaining additional purchased storage (current size + other project excess use some additional purchased storage)' do
+            let(:current_size) { limit + 1 }
 
-          it 'returns 1' do
-            expect(subject.exceeded_size).to eq(5.megabytes)
+            it 'returns zero' do
+              expect(subject.exceeded_size).to eq(0)
+            end
           end
-        end
-      end
 
-      context 'without additional purchased storage' do
-        context 'when change size will be over the limit' do
-          let(:current_size) { 50 }
+          context 'when additional purchased storage is depleted (current size + other project excess exceed additional purchased storage)' do
+            let(:total_repository_size_excess) { 15 }
+            let(:current_size) { 61 }
 
-          it 'returns 1' do
-            expect(subject.exceeded_size(1.megabytes)).to eq(1.megabytes)
+            it 'returns a positive number' do
+              expect(subject.exceeded_size).to eq(5.megabytes)
+            end
           end
         end
 
-        context 'when change size will not be over the limit' do
-          let(:current_size) { 49 }
+        context 'when a change size is provided' do
+          let(:change_size) { 1.megabyte }
+
+          context 'when current size + total repository size excess is below the limit (additional purchase storage not used)' do
+            let(:current_size) { limit - 1 }
 
-          it 'returns zero' do
-            expect(subject.exceeded_size(1.megabytes)).to eq(0)
+            it 'returns zero' do
+              expect(subject.exceeded_size(change_size)).to eq(0)
+            end
+          end
+
+          context 'when current size + total repository size excess is equal to the limit (additional purchase storage depleted)' do
+            let(:current_size) { limit }
+
+            it 'returns a positive number' do
+              expect(subject.exceeded_size(change_size)).to eq(1.megabyte)
+            end
           end
         end
       end
+
+      context 'without additional purchased storage' do
+        include_examples 'checker size exceeded'
+      end
     end
 
     context 'with feature flag :additional_repo_storage_by_namespace disabled' do

spec/support/shared_examples/lib/gitlab/repository_size_checker_shared_examples.rb

@@ -17,35 +17,58 @@ RSpec.shared_examples 'checker size not over limit' do
 end
 
 RSpec.shared_examples 'checker size exceeded' do
-  context 'when current size is below or equal to the limit' do
-    let(:current_size) { 50 }
+  context 'when no change size provided' do
+    context 'when current size is below the limit' do
+      let(:current_size) { limit - 1 }
 
-    it 'returns zero' do
-      expect(subject.exceeded_size).to eq(0)
+      it 'returns a negative number' do
+        expect(subject.exceeded_size).to eq(-1.megabyte)
+      end
     end
-  end
 
-  context 'when current size is over the limit' do
-    let(:current_size) { 51 }
+    context 'when current size is equal to the limit' do
+      let(:current_size) { limit }
 
-    it 'returns zero' do
-      expect(subject.exceeded_size).to eq(1.megabytes)
+      it 'returns zero' do
+        expect(subject.exceeded_size).to eq(0)
+      end
     end
-  end
 
-  context 'when change size will be over the limit' do
-    let(:current_size) { 50 }
+    context 'when current size is over the limit' do
+      let(:current_size) { limit + 1 }
+      let(:total_repository_size_excess) { 1 }
 
-    it 'returns zero' do
-      expect(subject.exceeded_size(1.megabytes)).to eq(1.megabytes)
+      it 'returns a positive number' do
+        expect(subject.exceeded_size).to eq(1.megabyte)
+      end
     end
   end
 
-  context 'when change size will not be over the limit' do
-    let(:current_size) { 49 }
+  context 'when a change size is provided' do
+    let(:change_size) { 1.megabyte }
+
+    context 'when change size will be over the limit' do
+      let(:current_size) { limit }
+
+      it 'returns a positive number' do
+        expect(subject.exceeded_size(change_size)).to eq(1.megabyte)
+      end
+    end
+
+    context 'when change size will be at the limit' do
+      let(:current_size) { limit - 1 }
+
+      it 'returns zero' do
+        expect(subject.exceeded_size(change_size)).to eq(0)
+      end
+    end
+
+    context 'when change size will be under the limit' do
+      let(:current_size) { limit - 2 }
 
-    it 'returns zero' do
-      expect(subject.exceeded_size(1.megabytes)).to eq(0)
+      it 'returns zero' do
+        expect(subject.exceeded_size(change_size)).to eq(-1.megabyte)
+      end
     end
   end
 end