[WIP] refactor linter specbvl-validate-po-files

4 files changed, 55 insertions, 26 deletions

diff --git a/lib/gitlab/po_linter.rb b/lib/gitlab/po_linter.rb
index abf048815a1..5f460807e61 100644
--- a/lib/gitlab/po_linter.rb
+++ b/lib/gitlab/po_linter.rb
@@ -38,14 +38,14 @@ module Gitlab
         # Skip validation of metadata
         next if entry[:msgid].empty?
 
-        errors_for_entry = errors_for_entry(entry)
+        errors_for_entry = validate_entry(entry)
         errors[join_message(entry[:msgid])] = errors_for_entry if errors_for_entry.any?
       end
 
       errors
     end
 
-    def errors_for_entry(entry)
+    def validate_entry(entry)
       errors = []
 
       validate_flags(errors, entry)
@@ -64,20 +64,16 @@ module Gitlab
     end
 
     def validate_variables_in_message(errors, message_id, message_translation)
-      # An empty translation is fine, we fall back to English
-      return unless message_translation.present?
-
       message_id = join_message(message_id)
-      message_translation = join_message(message_translation)
-
       used_variables = message_id.scan(VARIABLE_REGEX)
 
       validate_unnamed_variables(errors, used_variables)
-
-      return if message_translation.empty?
-
-      validate_variable_usage(errors, message_translation, used_variables)
       validate_translation(errors, message_id, used_variables)
+
+      message_translation = join_message(message_translation)
+      unless message_translation.empty?
+        validate_variable_usage(errors, message_translation, used_variables)
+      end
     end
 
     def validate_translation(errors, message_id, used_variables)
@@ -94,11 +90,13 @@ module Gitlab
       if variables.empty?
         []
       elsif variables.any? { |variable| unnamed_variable?(variable) }
-        variables.map { |variable| variable == '%d' ? Random.rand : SecureRandom.hex }
+        variables.map do |variable|
+          variable == '%d' ? Random.rand(1000) : Gitlab::Utils.random_string
+        end
       else
         variables.inject({}) do |hash, variable|
           variable_name = variable.match(/[^%{].*[^}]/).to_s
-          hash[variable_name] = SecureRandom.hex
+          hash[variable_name] = Gitlab::Utils.random_string
           hash
         end
       end
diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb
index fa182c4deda..92de4cf2c19 100644
--- a/lib/gitlab/utils.rb
+++ b/lib/gitlab/utils.rb
@@ -29,5 +29,9 @@ module Gitlab
         'No'
       end
     end
+
+    def random_string
+      Random.rand(Float::MAX.to_i).to_s(36)
+    end
   end
 end
diff --git a/spec/lib/gitlab/po_linter_spec.rb b/spec/lib/gitlab/po_linter_spec.rb
index 71e2e3ae323..4e16942a5b0 100644
--- a/spec/lib/gitlab/po_linter_spec.rb
+++ b/spec/lib/gitlab/po_linter_spec.rb
@@ -55,15 +55,15 @@ describe Gitlab::PoLinter do
   end
 
   describe '#parse_po' do
-    let!(:subject) { linter.parse_po }
-
     context 'with a valid po' do
       it 'fills in the entries' do
+        linter.parse_po
+
         expect(linter.entries).not_to be_empty
       end
 
       it 'does not have errors' do
-        is_expected.to be_nil
+        expect(linter.parse_po).to be_nil
       end
     end
 
@@ -71,11 +71,13 @@ describe Gitlab::PoLinter do
       let(:po_path) { 'spec/fixtures/invalid.po' }
 
       it 'contains an error' do
-        is_expected.not_to be_nil
+        expect(linter.parse_po).not_to be_nil
       end
 
       it 'sets the entries to an empty array' do
-        expect(linter.entries).to be_empty
+        linter.parse_po
+
+        expect(linter.entries).to eq([])
       end
     end
   end
@@ -84,16 +86,14 @@ describe Gitlab::PoLinter do
     it 'skips entries without a `msgid`' do
       allow(linter).to receive(:entries) { [{ msgid: "" }] }
 
-      expect(linter).not_to receive(:errors_for_entry)
-
-      linter.validate_entries
+      expect(linter.validate_entries).to be_empty
     end
 
     it 'keeps track of errors for entries' do
       fake_invalid_entry = { msgid: "Hello %{world}", msgstr: "Bonjour %{monde}" }
       allow(linter).to receive(:entries) { [fake_invalid_entry] }
 
-      expect(linter).to receive(:errors_for_entry)
+      expect(linter).to receive(:validate_entry)
                           .with(fake_invalid_entry)
                           .and_call_original
 
@@ -108,7 +108,7 @@ describe Gitlab::PoLinter do
       expect(linter).to receive(:validate_flags).with([], fake_entry)
       expect(linter).to receive(:validate_variables).with([], fake_entry)
 
-      linter.errors_for_entry(fake_entry)
+      linter.validate_entry(fake_entry)
     end
   end
 
@@ -154,6 +154,14 @@ describe Gitlab::PoLinter do
   end
 
   describe '#validate_translation' do
+    it 'succeeds with valid variables' do
+      errors = []
+
+      linter.validate_translation(errors, 'Hello %{world}',  ['%{world}'])
+
+      expect(errors).to be_empty
+    end
+
     it 'adds an error message when translating fails' do
       errors = []
 
@@ -163,14 +171,27 @@ describe Gitlab::PoLinter do
 
       expect(errors).to include('Failure translating to en with []: broken')
     end
+
+    it "adds an error when trying to translate with incorrect variables" do
+      errors = []
+
+      linter.validate_translation(errors, 'Hello %d', ['%s'])
+
+      expect(errors.first).to start_with("Failure translating to en with")
+    end
   end
 
   describe '#fill_in_variables' do
     it 'builds an array for %d translations' do
       result = linter.fill_in_variables(['%d'])
 
-      expect(result).to be_kind_of(Array)
-      expect(result).to include(an_instance_of(Float))
+      expect(result).to contain_exactly(a_kind_of(Integer))
+    end
+
+    it 'builds an array for %s translations' do
+      result = linter.fill_in_variables(['%s'])
+
+      expect(result).to contain_exactly(a_kind_of(String))
     end
 
     it 'builds a hash for named variables' do
diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb
index 00941aec380..4123f7af88d 100644
--- a/spec/lib/gitlab/utils_spec.rb
+++ b/spec/lib/gitlab/utils_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Utils, lib: true do
-  delegate :to_boolean, :boolean_to_yes_no, to: :described_class
+  delegate :to_boolean, :boolean_to_yes_no, :random_string, to: :described_class
 
   describe '.to_boolean' do
     it 'accepts booleans' do
@@ -39,4 +39,10 @@ describe Gitlab::Utils, lib: true do
       expect(boolean_to_yes_no(false)).to eq('No')
     end
   end
+
+  describe '.random_string' do
+    it 'generates a string' do
+      expect(random_string).to be_kind_of(String)
+    end
+  end
 end