diff --git a/CHANGELOG.md b/CHANGELOG.md index c636f3777..013c9f48f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ _None_ * Add the option for `an_localize_libs` to provide a `source_id` for each library being merged. If provided, that identifier will be added as an `a8c-src-lib` XML attribute to the `` nodes being updated with strings from said library. This can be useful to help identify where each string come from in the resulting, merged `strings.xml`. [#351] +* Add the option for `an_localize_libs` to set the `tools:ignore="UnusedResources"` XML attribute for each string being merged from a library. [#354] ### Bug Fixes diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/actions/android/an_localize_libs_action.rb b/lib/fastlane/plugin/wpmreleasetoolkit/actions/android/an_localize_libs_action.rb index 8c82c36f3..db90162dc 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/actions/android/an_localize_libs_action.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/actions/android/an_localize_libs_action.rb @@ -33,11 +33,12 @@ def self.details def self.available_options libs_hash_description = <<~KEYS - - `:library`: The library display name - - `:strings_path`: The path to the `strings.xml` file of the library - - `:exclusions`: An optional `Array` of string keys to exclude from merging + - `:library`: The library display name. + - `:strings_path`: The path to the `strings.xml` file of the library. + - `:exclusions`: An optional `Array` of string keys to exclude from merging. - `:source_id`: An optional `String` which will be added as the `a8c-src-lib` XML attribute to strings coming from this library, to help identify their source in the merged file. + - `:add_ignore_attr`: If set to true, will add `tools:ignore="UnusedResources"` to merged strings. KEYS [ FastlaneCore::ConfigItem.new(key: :app_strings_path, diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb index 02487f3b8..52e914c11 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb @@ -33,11 +33,20 @@ def self.skip_string_by_exclusion_list(library, string_name) end end + # Adds the appropriate XML attributes to an XML `` node according to library configuration + def self.add_xml_attributes!(string_node, library) + if library[:add_ignore_attr] == true + existing_ignores = (string_node['tools:ignore'] || '').split(',') + existing_ignores.append('UnusedResources') unless existing_ignores.include?('UnusedResources') + string_node['tools:ignore'] = existing_ignores.join(',') + end + string_node[LIB_SOURCE_XML_ATTR] = library[:source_id] unless library[:source_id].nil? + end + # Merge string_line into main_string def self.merge_string(main_strings, library, string_line) string_name = string_line.attr('name') string_content = string_line.content - lib_src_id = library[:source_id] # Skip strings in the exclusions list return :skipped if skip_string_by_exclusion_list(library, string_name) @@ -61,14 +70,14 @@ def self.merge_string(main_strings, library, string_line) else # It has the tools:ignore flag, so update the content without touching the other attributes this_string.content = string_content - this_string[LIB_SOURCE_XML_ATTR] = lib_src_id unless lib_src_id.nil? + add_xml_attributes!(this_string, library) return result end end end # String not found, or removed because needing update and not in the exclusion list: add to the main file - string_line[LIB_SOURCE_XML_ATTR] = lib_src_id unless lib_src_id.nil? + add_xml_attributes!(string_line, library) main_strings.xpath('//string').last().add_next_sibling("\n#{' ' * 4}#{string_line.to_xml().strip}") return result end @@ -103,8 +112,14 @@ def self.verify_string(main_strings, library, string_line) # @param [Hash] library Hash describing the library to merge. The Hash should contain the following keys: # - `:library`: The human readable name of the library, used to display in console messages # - `:strings_path`: The path to the strings.xml file of the library to merge into the main one - # - `:exclusions`: An array of strings keys to exclude during merge. Any of those keys from the library's `strings.xml` will be skipped and won't be merged into the main one. + # - `:exclusions`: An array of strings keys to exclude during merge. Any of those keys from the + # library's `strings.xml` will be skipped and won't be merged into the main one. + # - `:source_id`: An optional `String` which will be added as the `a8c-src-lib` XML attribute + # to strings coming from this library, to help identify their source in the merged file. + # - `:add_ignore_attr`: If set to `true`, will add `tools:ignore="UnusedResources"` to merged strings. + # # @return [Boolean] True if at least one string from the library has been added to (or has updated) the main strings file. + # def self.merge_lib(main, library) UI.message("Merging #{library[:library]} strings into #{main}") main_strings = File.open(main) { |f| Nokogiri::XML(f, nil, Encoding::UTF_8.to_s) } diff --git a/spec/an_localize_libs_action_spec.rb b/spec/an_localize_libs_action_spec.rb index 4316fec95..25f4b26d2 100644 --- a/spec/an_localize_libs_action_spec.rb +++ b/spec/an_localize_libs_action_spec.rb @@ -1,48 +1,223 @@ require 'spec_helper' describe Fastlane::Actions::AnLocalizeLibsAction do - # This test is more of a way of ensuring `run_described_fastlane_action` handles array of hashes properly than a comprehensive test for the `an_localize_libs_action` action. - # - # Please consider expanding this test if you'll find yourself working on its action. - it 'merges the strings from the given array into the given main strings file' do - in_tmp_dir do |tmp_dir| - app_strings_path = File.join(tmp_dir, 'app.xml') - File.write(app_strings_path, android_xml_with_content('test from app')) - - lib1_strings_path = File.join(tmp_dir, 'lib1.xml') - File.write(lib1_strings_path, android_xml_with_content('test from lib 1')) - - lib2_strings_path = File.join(tmp_dir, 'lib2.xml') - File.write(lib2_strings_path, android_xml_with_content('test from lib 2')) - - run_described_fastlane_action( - app_strings_path: app_strings_path, - libs_strings_path: [ - { library: 'lib_1', strings_path: lib1_strings_path, exclusions: [] }, - { library: 'lib_2', strings_path: lib2_strings_path, exclusions: [] }, - ] - ) - - expected = <<~XML - test from app - test from lib 1 - test from lib 2 - XML - expect(File.read(app_strings_path)).to eq(android_xml_with_content(expected)) - end + def android_xml_with_lines(lines) + # I couldn't find a way to interpolate a multiline string preserving its indentation in the heredoc below, so I hacked the following transformation of the input that adds the desired indentation to all lines. + # + # The desired indentation is 4 spaces to stay aligned with the production code applies when merging the XMLs. + indented_content = lines.map { |l| " #{l.chomp}" }.join("\n") + + return <<~XML + + + #{indented_content} + + XML end -end -def android_xml_with_content(content) - # I couldn't find a way to interpolate a multiline string preserving its indentation in the heredoc below, so I hacked the following transformation of the input that adds the desired indentation to all lines. - # - # The desired indentation is 4 spaces to stay aligned with the production code applies when merging the XMLs. - indented_content = content.chomp.lines.map { |l| " #{l}" }.join - - return <<~XML - - - #{indented_content} - - XML + def write_android_xml(path, lines) + File.write(path, android_xml_with_lines(lines)) + end + + describe 'merges the strings from the given array into the given main strings file' do + it 'handles simple XMLs with no duplicates nor attributes' do + in_tmp_dir do |tmp_dir| + app_strings_path = File.join(tmp_dir, 'app.xml') + File.write(app_strings_path, android_xml_with_lines(['test from app'])) + + lib1_strings_path = File.join(tmp_dir, 'lib1.xml') + File.write(lib1_strings_path, android_xml_with_lines(['test from lib 1'])) + + lib2_strings_path = File.join(tmp_dir, 'lib2.xml') + File.write(lib2_strings_path, android_xml_with_lines(['test from lib 2'])) + + run_described_fastlane_action( + app_strings_path: app_strings_path, + libs_strings_path: [ + { library: 'lib_1', strings_path: lib1_strings_path, exclusions: [] }, + { library: 'lib_2', strings_path: lib2_strings_path, exclusions: [] }, + ] + ) + + expected = [ + 'test from app', + 'test from lib 1', + 'test from lib 2', + ] + expect(File.read(app_strings_path)).to eq(android_xml_with_lines(expected)) + end + end + + it 'keeps app value if content_override is used' do + in_tmp_dir do |tmp_dir| + app_strings_path = File.join(tmp_dir, 'app.xml') + app_xml_lines = [ + 'from app override-true', + 'from app override-false', + 'from app override-missing', + ] + File.write(app_strings_path, android_xml_with_lines(app_xml_lines)) + + lib_strings_path = File.join(tmp_dir, 'lib.xml') + lib_xml_lines = [ + 'from lib override-true', + 'from lib override-false', + 'from lib override-missing', + ] + File.write(lib_strings_path, android_xml_with_lines(lib_xml_lines)) + + run_described_fastlane_action( + app_strings_path: app_strings_path, + libs_strings_path: [ + { library: 'lib', strings_path: lib_strings_path, exclusions: [] }, + ] + ) + + expected = [ + 'from app override-true', + '', '', # FIXME: Current implementation adds empty lines; we should get rid of those at some point + 'from lib override-false', + 'from lib override-missing', + ] + expect(File.read(app_strings_path)).to eq(android_xml_with_lines(expected)) + end + end + + it 'adds a8c-lib-src attribute if provided' do + in_tmp_dir do |tmp_dir| + app_strings_path = File.join(tmp_dir, 'app.xml') + app_xml_lines = [ + 'from app override-true', + 'from app override-missing', + ] + File.write(app_strings_path, android_xml_with_lines(app_xml_lines)) + + lib1_strings_path = File.join(tmp_dir, 'lib1.xml') + lib1_xml_lines = [ + 'from lib1 override-true', + 'from lib1 override-missing', + 'Key only present in lib1', + ] + File.write(lib1_strings_path, android_xml_with_lines(lib1_xml_lines)) + + lib2_strings_path = File.join(tmp_dir, 'lib2.xml') + lib2_xml_lines = [ + 'from lib2 override-true', + 'from lib2 override-missing', + 'Key only present in lib2', + ] + File.write(lib2_strings_path, android_xml_with_lines(lib2_xml_lines)) + + run_described_fastlane_action( + app_strings_path: app_strings_path, + libs_strings_path: [ + { library: 'lib1', strings_path: lib1_strings_path, source_id: 'lib1-id' }, + { library: 'lib2', strings_path: lib2_strings_path, source_id: 'lib2-id' }, + ] + ) + + expected = [ + 'from app override-true', + '', # FIXME: Current implementation adds empty lines; we should get rid of those at some point + 'Key only present in lib1', + 'from lib2 override-missing', + 'Key only present in lib2', + '', # FIXME: Current implementation adds empty lines; we should get rid of those at some point + ] + expect(File.read(app_strings_path)).to eq(android_xml_with_lines(expected)) + end + end + + it 'adds tools:ignore attribute when requested' do + in_tmp_dir do |tmp_dir| + app_strings_path = File.join(tmp_dir, 'app.xml') + app_xml_lines = [ + 'from app, override true', + 'from app, tools:ignore="UnusedResources"', + 'from app, tools:ignore mix', + 'from app, tools:ignore mix', + ] + File.write(app_strings_path, android_xml_with_lines(app_xml_lines)) + + lib1_strings_path = File.join(tmp_dir, 'lib1.xml') + lib1_xml_lines = [ + 'from lib1, override true', + 'Key only present in lib1, no extra attribute', + 'Key only present in lib1, with tools:ignore attribute', + 'Key only present in lib1, with tools:ignore attribute x,y', + 'Key only present in lib1, with tools:ignore attribute x,y', + ] + File.write(lib1_strings_path, android_xml_with_lines(lib1_xml_lines)) + + lib2_strings_path = File.join(tmp_dir, 'lib2.xml') + lib2_xml_lines = [ + 'from lib2, override true', + 'Key only present in lib2, no extra attribute', + 'Key only present in lib2, with tools:ignore attribute', + 'Key only present in lib2, with tools:ignore attribute x,y', + 'Key only present in lib2, with tools:ignore attribute x,y', + ] + File.write(lib2_strings_path, android_xml_with_lines(lib2_xml_lines)) + + run_described_fastlane_action( + app_strings_path: app_strings_path, + libs_strings_path: [ + { library: 'lib1', strings_path: lib1_strings_path, source_id: 'lib1', add_ignore_attr: true }, + { library: 'lib2', strings_path: lib2_strings_path, source_id: 'lib2' }, + ] + ) + + expected = [ + 'from app, override true', + 'from app, tools:ignore="UnusedResources"', + 'from app, tools:ignore mix', + 'from app, tools:ignore mix', + 'Key only present in lib1, no extra attribute', + 'Key only present in lib1, with tools:ignore attribute', + 'Key only present in lib1, with tools:ignore attribute x,y', + 'Key only present in lib1, with tools:ignore attribute x,y', + 'Key only present in lib2, no extra attribute', + 'Key only present in lib2, with tools:ignore attribute', + 'Key only present in lib2, with tools:ignore attribute x,y', + 'Key only present in lib2, with tools:ignore attribute x,y', + ] + expect(File.read(app_strings_path)).to eq(android_xml_with_lines(expected)) + end + end + + it 'handles exclusions list per library' do + in_tmp_dir do |tmp_dir| + app_strings_path = File.join(tmp_dir, 'app.xml') + app_xml_lines = [ + 'from app override-true', + 'from app override-false', + 'from app override-missing', + ] + File.write(app_strings_path, android_xml_with_lines(app_xml_lines)) + + lib_strings_path = File.join(tmp_dir, 'lib.xml') + lib_xml_lines = [ + 'from lib override-true', + 'from lib override-false', + 'from lib override-missing', + ] + File.write(lib_strings_path, android_xml_with_lines(lib_xml_lines)) + + run_described_fastlane_action( + app_strings_path: app_strings_path, + libs_strings_path: [ + { library: 'lib', strings_path: lib_strings_path, exclusions: ['override-missing'] }, + ] + ) + + expected = [ + 'from app override-true', + '', # FIXME: Current implementation adds empty lines; we should get rid of those at some point + 'from app override-missing', + 'from lib override-false', + ] + expect(File.read(app_strings_path)).to eq(android_xml_with_lines(expected)) + end + end + end end