kotovalexarian-likes-gitlab
/
gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Correctly translate all forms in tests

This commit is contained in:
Bob Van Landuyt 2018-05-14 15:28:39 +02:00
parent f5d45e0632
commit 17fc178cb5
7 changed files with 126 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
lib/gitlab/i18n/metadata_entry.rb
View File

@ -3,16 +3,25 @@ module Gitlab
class MetadataEntry
attr_reader :entry_data
# Avoid testing too many plurals if `nplurals` was incorrectly set.
# Based on info on https://www.gnu.org/software/gettext/manual/html_node/Plural-forms.html
# which mentions special cases for numbers ending in 2 digits
MAX_FORMS_TO_TEST = 101
def initialize(entry_data)
@entry_data = entry_data
end
def expected_plurals
def expected_forms
return nil unless plural_information
plural_information['nplurals'].to_i
end
def forms_to_test
@forms_to_test ||= [expected_forms, MAX_FORMS_TO_TEST].compact.min
end
private
def plural_information

61
lib/gitlab/i18n/po_linter.rb
View File

@ -36,7 +36,7 @@ module Gitlab
end
@translation_entries = entries.map do |entry_data|
Gitlab::I18n::TranslationEntry.new(entry_data, metadata_entry.expected_plurals)
Gitlab::I18n::TranslationEntry.new(entry_data, metadata_entry.expected_forms)
end
nil
@ -84,25 +84,25 @@ module Gitlab
end
def validate_number_of_plurals(errors, entry)
return unless metadata_entry&.expected_plurals
return unless metadata_entry&.expected_forms
return unless entry.translated?
if entry.has_plural? && entry.all_translations.size != metadata_entry.expected_plurals
errors << "should have #{metadata_entry.expected_plurals} "\
"#{'translations'.pluralize(metadata_entry.expected_plurals)}"
if entry.has_plural? && entry.all_translations.size != metadata_entry.expected_forms
errors << "should have #{metadata_entry.expected_forms} "\
"#{'translations'.pluralize(metadata_entry.expected_forms)}"
end
end
def validate_newlines(errors, entry)
if entry.msgid_contains_newlines?
if entry.msgid_has_multiple_lines?
errors << 'is defined over multiple lines, this breaks some tooling.'
end
if entry.plural_id_contains_newlines?
if entry.plural_id_has_multiple_lines?
errors << 'plural is defined over multiple lines, this breaks some tooling.'
end
if entry.translations_contain_newlines?
if entry.translations_have_multiple_lines?
errors << 'has translations defined over multiple lines, this breaks some tooling.'
end
end
@ -179,16 +179,41 @@ module Gitlab
end
def numbers_covering_all_plurals
@numbers_covering_all_plurals ||= Array.new(metadata_entry.expected_plurals) do |index|
number_for_pluralization(index)
end
@numbers_covering_all_plurals ||= calculate_numbers_covering_all_plurals
end
def number_for_pluralization(counter)
pluralization_result = FastGettext.pluralisation_rule.call(counter)
def calculate_numbers_covering_all_plurals
required_numbers = []
discovered_indexes = []
counter = 0
if pluralization_result.is_a?(TrueClass) || pluralization_result.is_a?(FalseClass)
counter
while discovered_indexes.size < metadata_entry.forms_to_test && counter < Gitlab::I18n::MetadataEntry::MAX_FORMS_TO_TEST
index_for_count = index_for_pluralization(counter)
unless discovered_indexes.include?(index_for_count)
discovered_indexes << index_for_count
required_numbers << counter
end
counter += 1
end
required_numbers
end
def index_for_pluralization(counter)
# This calls the C function that defines the pluralization rule, it can
# return a boolean (`false` represents 0, `true` represents 1) or an integer
# that specifies the plural form to be used for the given number
pluralization_result = Gitlab::I18n.with_locale(locale) do
FastGettext.pluralisation_rule.call(counter)
end
case pluralization_result
when false
0
when true
1
else
pluralization_result
end
@ -211,11 +236,13 @@ module Gitlab
end
def validate_unnamed_variables(errors, variables)
if variables.any? { |name| unnamed_variable?(name) } && variables.any? { |name| !unnamed_variable?(name) }
unnamed_variables, named_variables = variables.partition { |name| unnamed_variable?(name) }
if unnamed_variables.any? && named_variables.any?
errors << 'is combining named variables with unnamed variables'
end
if variables.select { |variable_name| unnamed_variable?(variable_name) }.size > 1
if unnamed_variables.size > 1
errors << 'is combining multiple unnamed variables'
end
end

6
lib/gitlab/i18n/translation_entry.rb
View File

@ -53,15 +53,15 @@ module Gitlab
nplurals > 1 || !has_plural?
end
def msgid_contains_newlines?
def msgid_has_multiple_lines?
entry_data[:msgid].is_a?(Array)
end
def plural_id_contains_newlines?
def plural_id_has_multiple_lines?
entry_data[:msgid_plural].is_a?(Array)
end
def translations_contain_newlines?
def translations_have_multiple_lines?
translation_entries.any? { |translation| translation.is_a?(Array) }
end

8
lib/tasks/gettext.rake
View File

@ -51,7 +51,7 @@ namespace :gettext do
end
task :updated_check do
# Removeing all pre-translated files speeds up `gettext:find` as the
# Removing all pre-translated files speeds up `gettext:find` as the
# files don't need to be merged.
`rm locale/*/gitlab.po`
@ -62,15 +62,15 @@ namespace :gettext do
changed_files = `git diff --name-only`.lines.map(&:strip)
# reset the locale folder for potential next tasks
`git checkout locale`
`git checkout -- locale`
if changed_files.include?('locale/gitlab.pot')
raise <<~MSG
Newly translated strings found, please add them to `gitlab.pot` by running:
bundle exec rake gettext:find; git checkout locale/*/gitlab.po;
bundle exec rake gettext:find; git checkout -- locale/*/gitlab.po;
Then check in the resulting `locale/gitlab.pot`
Then commit and push the resulting changes to `locale/gitlab.pot`.
MSG
end

6
spec/lib/gitlab/i18n/metadata_entry_spec.rb
View File

@ -1,7 +1,7 @@
require 'spec_helper'
describe Gitlab::I18n::MetadataEntry do
describe '#expected_plurals' do
describe '#expected_forms' do
it 'returns the number of plurals' do
data = {
msgid: "",
@ -22,7 +22,7 @@ describe Gitlab::I18n::MetadataEntry do
}
entry = described_class.new(data)
expect(entry.expected_plurals).to eq(2)
expect(entry.expected_forms).to eq(2)
end
it 'returns 0 for the POT-metadata' do
@ -45,7 +45,7 @@ describe Gitlab::I18n::MetadataEntry do
}
entry = described_class.new(data)
expect(entry.expected_plurals).to eq(0)
expect(entry.expected_forms).to eq(0)
end
end
end

73
spec/lib/gitlab/i18n/po_linter_spec.rb
View File

@ -1,20 +1,22 @@
require 'spec_helper'
require 'simple_po_parser'
# Disabling this cop to allow for multi-language examples in comments
# rubocop:disable Style/AsciiComments
describe Gitlab::I18n::PoLinter do
let(:linter) { described_class.new(po_path) }
let(:po_path) { 'spec/fixtures/valid.po' }
def fake_translation(id:, translation:, plural_id: nil, plurals: [])
data = { msgid: id, msgid_plural: plural_id }
def fake_translation(msgid:, translation:, plural_id: nil, plurals: [])
data = { msgid: msgid, msgid_plural: plural_id }
if plural_id
[translation, *plurals].each_with_index do |plural, index|
allow(FastGettext::Translation).to receive(:n_).with(plural_id, index).and_return(plural)
allow(FastGettext::Translation).to receive(:n_).with(msgid, plural_id, index).and_return(plural)
data.merge!("msgstr[#{index}]" => plural)
end
else
allow(FastGettext::Translation).to receive(:_).with(id).and_return(translation)
allow(FastGettext::Translation).to receive(:_).with(msgid).and_return(translation)
data[:msgstr] = translation
end
@ -174,7 +176,7 @@ describe Gitlab::I18n::PoLinter do
describe '#validate_entries' do
it 'keeps track of errors for entries' do
fake_invalid_entry = fake_translation(id: "Hello %{world}",
fake_invalid_entry = fake_translation(msgid: "Hello %{world}",
translation: "Bonjour %{monde}")
allow(linter).to receive(:translation_entries) { [fake_invalid_entry] }
@ -204,7 +206,7 @@ describe Gitlab::I18n::PoLinter do
describe '#validate_number_of_plurals' do
it 'validates when there are an incorrect number of translations' do
fake_metadata = double
allow(fake_metadata).to receive(:expected_plurals).and_return(2)
allow(fake_metadata).to receive(:expected_forms).and_return(2)
allow(linter).to receive(:metadata_entry).and_return(fake_metadata)
fake_entry = Gitlab::I18n::TranslationEntry.new(
@ -226,7 +228,7 @@ describe Gitlab::I18n::PoLinter do
it 'validates both singular and plural in a pluralized string when the entry has a singular' do
pluralized_entry = fake_translation(
id: 'Hello %{world}',
msgid: 'Hello %{world}',
translation: 'Bonjour %{world}',
plural_id: 'Hello all %{world}',
plurals: ['Bonjour tous %{world}']
@ -244,7 +246,7 @@ describe Gitlab::I18n::PoLinter do
it 'only validates plural when there is no separate singular' do
pluralized_entry = fake_translation(
id: 'Hello %{world}',
msgid: 'Hello %{world}',
translation: 'Bonjour %{world}',
plural_id: 'Hello all %{world}'
)
@ -256,7 +258,7 @@ describe Gitlab::I18n::PoLinter do
end
it 'validates the message variables' do
entry = fake_translation(id: 'Hello', translation: 'Bonjour')
entry = fake_translation(msgid: 'Hello', translation: 'Bonjour')
expect(linter).to receive(:validate_variables_in_message)
.with([], 'Hello', 'Bonjour')
@ -266,7 +268,7 @@ describe Gitlab::I18n::PoLinter do
it 'validates variable usage in message ids' do
entry = fake_translation(
id: 'Hello %{world}',
msgid: 'Hello %{world}',
translation: 'Bonjour %{world}',
plural_id: 'Hello all %{world}',
plurals: ['Bonjour tous %{world}']
@ -309,7 +311,7 @@ describe Gitlab::I18n::PoLinter do
end
describe '#validate_translation' do
let(:entry) { fake_translation(id: 'Hello %{world}', translation: 'Bonjour %{world}') }
let(:entry) { fake_translation(msgid: 'Hello %{world}', translation: 'Bonjour %{world}') }
it 'succeeds with valid variables' do
errors = []
@ -330,7 +332,7 @@ describe Gitlab::I18n::PoLinter do
end
it 'adds an error message when translating fails when translating with context' do
entry = fake_translation(id: 'Tests|Hello', translation: 'broken')
entry = fake_translation(msgid: 'Tests|Hello', translation: 'broken')
errors = []
expect(FastGettext::Translation).to receive(:s_) { raise 'broken' }
@ -341,7 +343,7 @@ describe Gitlab::I18n::PoLinter do
end
it "adds an error when trying to translate with incorrect variables when using unnamed variables" do
entry = fake_translation(id: 'Hello %s', translation: 'Hello %d')
entry = fake_translation(msgid: 'Hello %s', translation: 'Hello %d')
errors = []
linter.validate_translation(errors, entry)
@ -350,13 +352,55 @@ describe Gitlab::I18n::PoLinter do
end
it "adds an error when trying to translate with named variables when unnamed variables are expected" do
entry = fake_translation(id: 'Hello %s', translation: 'Hello %{thing}')
entry = fake_translation(msgid: 'Hello %s', translation: 'Hello %{thing}')
errors = []
linter.validate_translation(errors, entry)
expect(errors.first).to start_with("Failure translating to en")
end
it 'tests translation for all given forms' do
# Fake a language that has 3 forms to translate
fake_metadata = double
allow(fake_metadata).to receive(:forms_to_test).and_return(3)
allow(linter).to receive(:metadata_entry).and_return(fake_metadata)
entry = fake_translation(
msgid: '%d exception',
translation: '%d uitzondering',
plural_id: '%d exceptions',
plurals: ['%d uitzonderingen', '%d uitzonderingetjes']
)
# Make each count use a different index
allow(linter).to receive(:index_for_pluralization).with(0).and_return(0)
allow(linter).to receive(:index_for_pluralization).with(1).and_return(1)
allow(linter).to receive(:index_for_pluralization).with(2).and_return(2)
expect(FastGettext::Translation).to receive(:n_).with('%d exception', '%d exceptions', 0).and_call_original
expect(FastGettext::Translation).to receive(:n_).with('%d exception', '%d exceptions', 1).and_call_original
expect(FastGettext::Translation).to receive(:n_).with('%d exception', '%d exceptions', 2).and_call_original
linter.validate_translation([], entry)
end
end
describe '#numbers_covering_all_plurals' do
it 'can correctly find all required numbers to translate to Polish' do
# Polish used as an example with 3 different forms:
# 0, all plurals except the ones ending in 2,3,4: Kotów
# 1: Kot
# 2-3-4: Koty
# So translating with [0, 1, 2] will give us all different posibilities
fake_metadata = double
allow(fake_metadata).to receive(:forms_to_test).and_return(4)
allow(linter).to receive(:metadata_entry).and_return(fake_metadata)
allow(linter).to receive(:locale).and_return('pl_PL')
numbers = linter.numbers_covering_all_plurals
expect(numbers).to contain_exactly(0, 1, 2)
end
end
describe '#fill_in_variables' do
@ -380,3 +424,4 @@ describe Gitlab::I18n::PoLinter do
end
end
end
# rubocop:enable Style/AsciiComments

6
spec/lib/gitlab/i18n/translation_entry_spec.rb
View File

@ -109,7 +109,7 @@ describe Gitlab::I18n::TranslationEntry do
data = { msgid: %w(hello world) }
entry = described_class.new(data, 2)
expect(entry.msgid_contains_newlines?).to be_truthy
expect(entry.msgid_has_multiple_lines?).to be_truthy
end
end
@ -118,7 +118,7 @@ describe Gitlab::I18n::TranslationEntry do
data = { msgid_plural: %w(hello world) }
entry = described_class.new(data, 2)
expect(entry.plural_id_contains_newlines?).to be_truthy
expect(entry.plural_id_has_multiple_lines?).to be_truthy
end
end
@ -127,7 +127,7 @@ describe Gitlab::I18n::TranslationEntry do
data = { msgstr: %w(hello world) }
entry = described_class.new(data, 2)
expect(entry.translations_contain_newlines?).to be_truthy
expect(entry.translations_have_multiple_lines?).to be_truthy
end
end