Skip to content

Commit

Permalink
Merge pull request #354 from wordpress-mobile/merge-android-strings-t…
Browse files Browse the repository at this point in the history
…oolsignore

Allow auto-addition of `tools:ignore="UnusedResources"` when merging Android strings
  • Loading branch information
AliSoftware authored Apr 20, 2022
2 parents 0e84ac5 + ba4a46c commit 9ba87e4
Show file tree
Hide file tree
Showing 4 changed files with 241 additions and 49 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<string>` 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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,20 @@ def self.skip_string_by_exclusion_list(library, string_name)
end
end

# Adds the appropriate XML attributes to an XML `<string>` 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)
Expand All @@ -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
Expand Down Expand Up @@ -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) }
Expand Down
259 changes: 217 additions & 42 deletions spec/an_localize_libs_action_spec.rb
Original file line number Diff line number Diff line change
@@ -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('<string name="a_string">test from app</string>'))

lib1_strings_path = File.join(tmp_dir, 'lib1.xml')
File.write(lib1_strings_path, android_xml_with_content('<string name="a_lib1_string">test from lib 1</string>'))

lib2_strings_path = File.join(tmp_dir, 'lib2.xml')
File.write(lib2_strings_path, android_xml_with_content('<string name="a_lib2_string">test from lib 2</string>'))

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
<string name="a_string">test from app</string>
<string name="a_lib1_string">test from lib 1</string>
<string name="a_lib2_string">test from lib 2</string>
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
<?xml version="1.0" encoding="UTF-8"?>
<resources xmlns:tools="http://schemas.android.com/tools">
#{indented_content}
</resources>
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
<?xml version="1.0" encoding="UTF-8"?>
<resources xmlns:tools="http://schemas.android.com/tools">
#{indented_content}
</resources>
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(['<string name="a_string">test from app</string>']))

lib1_strings_path = File.join(tmp_dir, 'lib1.xml')
File.write(lib1_strings_path, android_xml_with_lines(['<string name="a_lib1_string">test from lib 1</string>']))

lib2_strings_path = File.join(tmp_dir, 'lib2.xml')
File.write(lib2_strings_path, android_xml_with_lines(['<string name="a_lib2_string">test from lib 2</string>']))

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 = [
'<string name="a_string">test from app</string>',
'<string name="a_lib1_string">test from lib 1</string>',
'<string name="a_lib2_string">test from lib 2</string>',
]
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 = [
'<string name="override-true" content_override="true">from app override-true</string>',
'<string name="override-false" content_override="false">from app override-false</string>',
'<string name="override-missing">from app override-missing</string>',
]
File.write(app_strings_path, android_xml_with_lines(app_xml_lines))

lib_strings_path = File.join(tmp_dir, 'lib.xml')
lib_xml_lines = [
'<string name="override-true">from lib override-true</string>',
'<string name="override-false">from lib override-false</string>',
'<string name="override-missing">from lib override-missing</string>',
]
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 = [
'<string name="override-true" content_override="true">from app override-true</string>',
'', '', # FIXME: Current implementation adds empty lines; we should get rid of those at some point
'<string name="override-false">from lib override-false</string>',
'<string name="override-missing">from lib override-missing</string>',
]
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 = [
'<string name="override-true" content_override="true">from app override-true</string>',
'<string name="override-missing">from app override-missing</string>',
]
File.write(app_strings_path, android_xml_with_lines(app_xml_lines))

lib1_strings_path = File.join(tmp_dir, 'lib1.xml')
lib1_xml_lines = [
'<string name="override-true">from lib1 override-true</string>',
'<string name="override-missing">from lib1 override-missing</string>',
'<string name="lib1-key">Key only present in lib1</string>',
]
File.write(lib1_strings_path, android_xml_with_lines(lib1_xml_lines))

lib2_strings_path = File.join(tmp_dir, 'lib2.xml')
lib2_xml_lines = [
'<string name="override-true">from lib2 override-true</string>',
'<string name="override-missing">from lib2 override-missing</string>',
'<string name="lib2-key">Key only present in lib2</string>',
]
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 = [
'<string name="override-true" content_override="true">from app override-true</string>',
'', # FIXME: Current implementation adds empty lines; we should get rid of those at some point
'<string name="lib1-key" a8c-src-lib="lib1-id">Key only present in lib1</string>',
'<string name="override-missing" a8c-src-lib="lib2-id">from lib2 override-missing</string>',
'<string name="lib2-key" a8c-src-lib="lib2-id">Key only present in lib2</string>',
'', # 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 = [
'<string name="override-true" content_override="true">from app, override true</string>',
'<string name="ignore-unused" tools:ignore="UnusedResources">from app, tools:ignore="UnusedResources"</string>',
'<string name="ignore-x-unused-y" tools:ignore="X,UnusedResources,Y">from app, tools:ignore mix</string>',
'<string name="ignore-x-y" tools:ignore="X,Y">from app, tools:ignore mix</string>',
]
File.write(app_strings_path, android_xml_with_lines(app_xml_lines))

lib1_strings_path = File.join(tmp_dir, 'lib1.xml')
lib1_xml_lines = [
'<string name="override-true">from lib1, override true</string>',
'<string name="lib1-key">Key only present in lib1, no extra attribute</string>',
'<string name="lib1-ignore-unused" tools:ignore="UnusedResources">Key only present in lib1, with tools:ignore attribute</string>',
'<string name="lib1-ignore-x-y" tools:ignore="X,Y">Key only present in lib1, with tools:ignore attribute x,y</string>',
'<string name="lib1-ignore-x-unused-y" tools:ignore="X,UnusedResources,Y">Key only present in lib1, with tools:ignore attribute x,y</string>',
]
File.write(lib1_strings_path, android_xml_with_lines(lib1_xml_lines))

lib2_strings_path = File.join(tmp_dir, 'lib2.xml')
lib2_xml_lines = [
'<string name="override-true">from lib2, override true</string>',
'<string name="lib2-key">Key only present in lib2, no extra attribute</string>',
'<string name="lib2-ignore-unused" tools:ignore="UnusedResources">Key only present in lib2, with tools:ignore attribute</string>',
'<string name="lib2-ignore-x-y" tools:ignore="X,Y">Key only present in lib2, with tools:ignore attribute x,y</string>',
'<string name="lib2-ignore-x-unused-y" tools:ignore="X,UnusedResources,Y">Key only present in lib2, with tools:ignore attribute x,y</string>',
]
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 = [
'<string name="override-true" content_override="true">from app, override true</string>',
'<string name="ignore-unused" tools:ignore="UnusedResources">from app, tools:ignore="UnusedResources"</string>',
'<string name="ignore-x-unused-y" tools:ignore="X,UnusedResources,Y">from app, tools:ignore mix</string>',
'<string name="ignore-x-y" tools:ignore="X,Y">from app, tools:ignore mix</string>',
'<string name="lib1-key" tools:ignore="UnusedResources" a8c-src-lib="lib1">Key only present in lib1, no extra attribute</string>',
'<string name="lib1-ignore-unused" tools:ignore="UnusedResources" a8c-src-lib="lib1">Key only present in lib1, with tools:ignore attribute</string>',
'<string name="lib1-ignore-x-y" tools:ignore="X,Y,UnusedResources" a8c-src-lib="lib1">Key only present in lib1, with tools:ignore attribute x,y</string>',
'<string name="lib1-ignore-x-unused-y" tools:ignore="X,UnusedResources,Y" a8c-src-lib="lib1">Key only present in lib1, with tools:ignore attribute x,y</string>',
'<string name="lib2-key" a8c-src-lib="lib2">Key only present in lib2, no extra attribute</string>',
'<string name="lib2-ignore-unused" tools:ignore="UnusedResources" a8c-src-lib="lib2">Key only present in lib2, with tools:ignore attribute</string>',
'<string name="lib2-ignore-x-y" tools:ignore="X,Y" a8c-src-lib="lib2">Key only present in lib2, with tools:ignore attribute x,y</string>',
'<string name="lib2-ignore-x-unused-y" tools:ignore="X,UnusedResources,Y" a8c-src-lib="lib2">Key only present in lib2, with tools:ignore attribute x,y</string>',
]
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 = [
'<string name="override-true" content_override="true">from app override-true</string>',
'<string name="override-false" content_override="false">from app override-false</string>',
'<string name="override-missing">from app override-missing</string>',
]
File.write(app_strings_path, android_xml_with_lines(app_xml_lines))

lib_strings_path = File.join(tmp_dir, 'lib.xml')
lib_xml_lines = [
'<string name="override-true">from lib override-true</string>',
'<string name="override-false">from lib override-false</string>',
'<string name="override-missing">from lib override-missing</string>',
]
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 = [
'<string name="override-true" content_override="true">from app override-true</string>',
'', # FIXME: Current implementation adds empty lines; we should get rid of those at some point
'<string name="override-missing">from app override-missing</string>',
'<string name="override-false">from lib override-false</string>',
]
expect(File.read(app_strings_path)).to eq(android_xml_with_lines(expected))
end
end
end
end

0 comments on commit 9ba87e4

Please sign in to comment.