From 18fbc8ccff0072d04ae9a8ae6defc4ee77505d45 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Fri, 28 Jan 2022 12:45:24 +0100 Subject: [PATCH 01/10] Switch to running `an_` and `gp_` metadata actions via Fastfile parsing --- ...mples_for_update_metadata_source_action.rb | 36 ++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/spec/shared_examples_for_update_metadata_source_action.rb b/spec/shared_examples_for_update_metadata_source_action.rb index 2c82949df..984b32811 100644 --- a/spec/shared_examples_for_update_metadata_source_action.rb +++ b/spec/shared_examples_for_update_metadata_source_action.rb @@ -19,8 +19,9 @@ file_2_path = File.join(dir, '2.txt') File.write(file_2_path, 'value 2') - described_class.run( + run_described_action( po_file_path: output_path, + release_version: '1.0', source_files: { key1: file_1_path, key2: file_2_path @@ -56,7 +57,7 @@ whats_new_path = File.join(dir, 'whats_new.txt') File.write(whats_new_path, "- something new\n- something else new") - described_class.run( + run_described_action( po_file_path: output_path, release_version: '1.23', source_files: { @@ -94,8 +95,9 @@ file_2_path = File.join(dir, '2.txt') File.write(file_2_path, 'value 2') - described_class.run( + run_described_action( po_file_path: output_path, + release_version: '1.0', source_files: { key1: file_1_path, key2: file_2_path @@ -135,7 +137,7 @@ release_notes_path = File.join(dir, 'release_notes.txt') File.write(release_notes_path, "- release notes\n- more release notes") - described_class.run( + run_described_action( po_file_path: output_path, release_version: '1.23', source_files: { @@ -159,3 +161,29 @@ end end end + +def run_described_action(parameters) + lane_name = 'test' + lane = <<~LANE + lane :#{lane_name} do + #{described_class.action_name}( + #{stringify_for_fastlane(parameters)} + ) + end + LANE + Fastlane::FastFile.new.parse(lane).runner.execute(lane_name.to_sym) +end + +def stringify_for_fastlane(hash) + hash.map do |key, value| + # rubocop:disable Style/CaseLikeIf + if value.is_a?(Hash) + "#{key}: {\n#{stringify_for_fastlane(value)}\n}" + elsif value.is_a?(String) + "#{key}: \"#{value}\"" + else + "#{key}: #{value}" + end + # rubocop:enable Style/CaseLikeIf + end.join(",\n") +end From f141e7b3e0684189e4c87eab2eb5750738f6ee49 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Fri, 28 Jan 2022 16:12:15 +0100 Subject: [PATCH 02/10] Move `run_described_action` method to `spec_helper.rb` --- ...mples_for_update_metadata_source_action.rb | 26 ---------------- spec/spec_helper.rb | 30 +++++++++++++++++++ 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/spec/shared_examples_for_update_metadata_source_action.rb b/spec/shared_examples_for_update_metadata_source_action.rb index 984b32811..f4df86eee 100644 --- a/spec/shared_examples_for_update_metadata_source_action.rb +++ b/spec/shared_examples_for_update_metadata_source_action.rb @@ -161,29 +161,3 @@ end end end - -def run_described_action(parameters) - lane_name = 'test' - lane = <<~LANE - lane :#{lane_name} do - #{described_class.action_name}( - #{stringify_for_fastlane(parameters)} - ) - end - LANE - Fastlane::FastFile.new.parse(lane).runner.execute(lane_name.to_sym) -end - -def stringify_for_fastlane(hash) - hash.map do |key, value| - # rubocop:disable Style/CaseLikeIf - if value.is_a?(Hash) - "#{key}: {\n#{stringify_for_fastlane(value)}\n}" - elsif value.is_a?(String) - "#{key}: \"#{value}\"" - else - "#{key}: #{value}" - end - # rubocop:enable Style/CaseLikeIf - end.join(",\n") -end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9fc311a64..b82fdd286 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -67,6 +67,20 @@ def expect_shell_command(*command, exitstatus: 0, output: '') expect(Open3).to receive(:popen2e).with(*command).and_yield(mock_input, mock_output, mock_thread) end +# If the `described_class` of a spec is a `Fastlane::Action` subclass, it runs +# it with the given parameters. +# +def run_described_action(parameters, lane_name = 'test') + lane = <<~LANE + lane :#{lane_name} do + #{described_class.action_name}( + #{stringify_for_fastlane(parameters)} + ) + end + LANE + Fastlane::FastFile.new.parse(lane).runner.execute(lane_name.to_sym) +end + # Executes the given block within an ad hoc temporary directory. def in_tmp_dir Dir.mktmpdir('a8c-release-toolkit-tests-') do |tmpdir| @@ -75,3 +89,19 @@ def in_tmp_dir end end end + +private + +def stringify_for_fastlane(hash) + hash.map do |key, value| + # rubocop:disable Style/CaseLikeIf + if value.is_a?(Hash) + "#{key}: {\n#{stringify_for_fastlane(value)}\n}" + elsif value.is_a?(String) + "#{key}: \"#{value}\"" + else + "#{key}: #{value}" + end + # rubocop:enable Style/CaseLikeIf + end.join(",\n") +end From f53fe80a5fe67ac9c58bb8ed148fc276e9a3ae98 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Fri, 28 Jan 2022 16:16:06 +0100 Subject: [PATCH 03/10] Use new `run_describe_action` in `ios_merge_strings_files_spec.rb` --- spec/ios_merge_strings_files_spec.rb | 64 ++++++++++++++-------------- 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/spec/ios_merge_strings_files_spec.rb b/spec/ios_merge_strings_files_spec.rb index cba549a18..2b5a0274e 100644 --- a/spec/ios_merge_strings_files_spec.rb +++ b/spec/ios_merge_strings_files_spec.rb @@ -1,41 +1,39 @@ -describe Fastlane do - describe Fastlane::FastFile do - let(:test_data_dir) { File.join(File.dirname(__FILE__), 'test-data', 'translations', 'ios_l10n_helper') } +describe Fastlane::Actions::IosMergeStringsFilesAction do + let(:test_data_dir) { File.join(File.dirname(__FILE__), 'test-data', 'translations', 'ios_l10n_helper') } - def fixture(name) - File.join(test_data_dir, name) - end - - describe '#ios_merge_strings_files' do - it 'calls the action with the proper parameters and warn and return duplicate keys' do - # Arrange - messages = [] - allow(FastlaneCore::UI).to receive(:important) do |message| - messages.append(message) - end - inputs = ['Localizable-utf16.strings', 'non-latin-utf16.strings'] + def fixture(name) + File.join(test_data_dir, name) + end - Dir.mktmpdir('a8c-release-toolkit-tests-') do |tmpdir| - inputs.each { |f| FileUtils.cp(fixture(f), tmpdir) } + describe '#ios_merge_strings_files' do + it 'calls the action with the proper parameters and warn and return duplicate keys' do + # Arrange + messages = [] + allow(FastlaneCore::UI).to receive(:important) do |message| + messages.append(message) + end + inputs = ['Localizable-utf16.strings', 'non-latin-utf16.strings'] - # Act - result = Dir.chdir(tmpdir) do - described_class.new.parse("lane :test do - ios_merge_strings_files( - paths: ['#{inputs[0]}', '#{inputs[1]}'], - destination: 'output.strings' - ) - end").runner.execute(:test) - end + Dir.mktmpdir('a8c-release-toolkit-tests-') do |tmpdir| + inputs.each { |f| FileUtils.cp(fixture(f), tmpdir) } - # Assert - expect(File).to exist(File.join(tmpdir, 'output.strings')) - expect(result).to eq(%w[key1 key2]) - expect(messages).to eq([ - 'Duplicate key found while merging the `.strings` files: `key1`', - 'Duplicate key found while merging the `.strings` files: `key2`', - ]) + # Act + result = Dir.chdir(tmpdir) do + run_described_action( + paths: [inputs[0], inputs[1]], + destination: 'output.strings' + ) end + + # Assert + expect(File).to exist(File.join(tmpdir, 'output.strings')) + expect(result).to eq(%w[key1 key2]) + expect(messages).to eq( + [ + 'Duplicate key found while merging the `.strings` files: `key1`', + 'Duplicate key found while merging the `.strings` files: `key2`', + ] + ) end end end From bb2768b16ae74e3e21798a4cdd956282a5aefd86 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Fri, 28 Jan 2022 16:37:42 +0100 Subject: [PATCH 04/10] Add expectation so `run_describe_action` is called only from actions --- spec/spec_helper.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b82fdd286..f941bb24c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -71,6 +71,8 @@ def expect_shell_command(*command, exitstatus: 0, output: '') # it with the given parameters. # def run_described_action(parameters, lane_name = 'test') + expect(described_class).to be < Fastlane::Action, "Only call `#{__callee__}` from a spec describing a `Fastlane::Action` subclass." + lane = <<~LANE lane :#{lane_name} do #{described_class.action_name}( From f3bdab39a11d518caa2c30853ec5e125168f6128 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Tue, 1 Feb 2022 12:51:32 +0100 Subject: [PATCH 05/10] Replace custom method with `#inspect` call This should be more robust and require no extra maintenance on our end. See discussion at https://github.com/wordpress-mobile/release-toolkit/pull/330#pullrequestreview-866360216 --- spec/spec_helper.rb | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f941bb24c..f9b021a3a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -76,7 +76,7 @@ def run_described_action(parameters, lane_name = 'test') lane = <<~LANE lane :#{lane_name} do #{described_class.action_name}( - #{stringify_for_fastlane(parameters)} + #{parameters.inspect} ) end LANE @@ -91,19 +91,3 @@ def in_tmp_dir end end end - -private - -def stringify_for_fastlane(hash) - hash.map do |key, value| - # rubocop:disable Style/CaseLikeIf - if value.is_a?(Hash) - "#{key}: {\n#{stringify_for_fastlane(value)}\n}" - elsif value.is_a?(String) - "#{key}: \"#{value}\"" - else - "#{key}: #{value}" - end - # rubocop:enable Style/CaseLikeIf - end.join(",\n") -end From 5cf9dd7e8691aa330e3c40839e6710c2226a4e49 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Tue, 1 Feb 2022 16:57:33 +0100 Subject: [PATCH 06/10] Add one test to cover `an_localize_libs_action` The main reason for this is to verify that the new implementation of the method to run an action in the tests from f3bdab39a11d518caa2c30853ec5e125168f6128 works with an `Array` of `Hash`. --- .rubocop_todo.yml | 1 + spec/an_localize_libs_action_spec.rb | 54 ++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 spec/an_localize_libs_action_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2dea29051..0c088ef80 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -131,6 +131,7 @@ RSpec/FilePath: - 'spec/android_localize_helper_spec.rb' - 'spec/android_merge_translators_strings_spec.rb' - 'spec/android_version_helper_spec.rb' + - 'spec/an_localize_libs_action_spec.rb' - 'spec/an_metadata_update_helper_spec.rb' - 'spec/an_update_metadata_source_spec.rb' - 'spec/configuration_spec.rb' diff --git a/spec/an_localize_libs_action_spec.rb b/spec/an_localize_libs_action_spec.rb new file mode 100644 index 000000000..2bd898bf2 --- /dev/null +++ b/spec/an_localize_libs_action_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +describe Fastlane::Actions::AnLocalizeLibsAction do + # This test is more of a way of ensuring `run_described_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_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: [] }, + ] + ) + + # Notice the extra indentation in the library strings. The action doesn't + # modify the app's strings content indentation, but it applies its own + # standard to the values read from the given library strings + 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 + 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. + indented_content = content.chomp.lines.map { |l| " #{l}" }.join + + return <<~XML + + + #{indented_content} + + XML +end From 635f9cad73653fb4db59a7962a25981616473283 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Tue, 1 Feb 2022 17:03:41 +0100 Subject: [PATCH 07/10] Rename `run_described_action` into `run_described_fastlane_action` This is for extra clarity. Thanks @AliSoftware, see: https://github.com/wordpress-mobile/release-toolkit/pull/330#discussion_r794650236 --- spec/an_localize_libs_action_spec.rb | 4 ++-- spec/ios_merge_strings_files_spec.rb | 2 +- spec/shared_examples_for_update_metadata_source_action.rb | 8 ++++---- spec/spec_helper.rb | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/an_localize_libs_action_spec.rb b/spec/an_localize_libs_action_spec.rb index 2bd898bf2..86455e3a7 100644 --- a/spec/an_localize_libs_action_spec.rb +++ b/spec/an_localize_libs_action_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Fastlane::Actions::AnLocalizeLibsAction do - # This test is more of a way of ensuring `run_described_action` handles array + # 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. # @@ -18,7 +18,7 @@ lib2_strings_path = File.join(tmp_dir, 'lib2.xml') File.write(lib2_strings_path, android_xml_with_content('test from lib 2')) - run_described_action( + run_described_fastlane_action( app_strings_path: app_strings_path, libs_strings_path: [ { library: 'lib_1', strings_path: lib1_strings_path, exclusions: [] }, diff --git a/spec/ios_merge_strings_files_spec.rb b/spec/ios_merge_strings_files_spec.rb index 2b5a0274e..22bd4cc2d 100644 --- a/spec/ios_merge_strings_files_spec.rb +++ b/spec/ios_merge_strings_files_spec.rb @@ -19,7 +19,7 @@ def fixture(name) # Act result = Dir.chdir(tmpdir) do - run_described_action( + run_described_fastlane_action( paths: [inputs[0], inputs[1]], destination: 'output.strings' ) diff --git a/spec/shared_examples_for_update_metadata_source_action.rb b/spec/shared_examples_for_update_metadata_source_action.rb index f4df86eee..d096096e2 100644 --- a/spec/shared_examples_for_update_metadata_source_action.rb +++ b/spec/shared_examples_for_update_metadata_source_action.rb @@ -19,7 +19,7 @@ file_2_path = File.join(dir, '2.txt') File.write(file_2_path, 'value 2') - run_described_action( + run_described_fastlane_action( po_file_path: output_path, release_version: '1.0', source_files: { @@ -57,7 +57,7 @@ whats_new_path = File.join(dir, 'whats_new.txt') File.write(whats_new_path, "- something new\n- something else new") - run_described_action( + run_described_fastlane_action( po_file_path: output_path, release_version: '1.23', source_files: { @@ -95,7 +95,7 @@ file_2_path = File.join(dir, '2.txt') File.write(file_2_path, 'value 2') - run_described_action( + run_described_fastlane_action( po_file_path: output_path, release_version: '1.0', source_files: { @@ -137,7 +137,7 @@ release_notes_path = File.join(dir, 'release_notes.txt') File.write(release_notes_path, "- release notes\n- more release notes") - run_described_action( + run_described_fastlane_action( po_file_path: output_path, release_version: '1.23', source_files: { diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f9b021a3a..a2198e746 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -70,7 +70,7 @@ def expect_shell_command(*command, exitstatus: 0, output: '') # If the `described_class` of a spec is a `Fastlane::Action` subclass, it runs # it with the given parameters. # -def run_described_action(parameters, lane_name = 'test') +def run_described_fastlane_action(parameters, lane_name = 'test') expect(described_class).to be < Fastlane::Action, "Only call `#{__callee__}` from a spec describing a `Fastlane::Action` subclass." lane = <<~LANE From 135fdb1485479fad8958683b04b8104cb04628a8 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Tue, 1 Feb 2022 17:10:52 +0100 Subject: [PATCH 08/10] Adopt new `run_described_fastlane_action` across all tests This is for both consistency and to ensure the implementation can handle different input parameters scenarios. --- spec/android_merge_translators_strings_spec.rb | 2 +- spec/ios_lint_localizations_spec.rb | 6 +++--- spec/ios_merge_translators_strings_spec.rb | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/android_merge_translators_strings_spec.rb b/spec/android_merge_translators_strings_spec.rb index f11c7e8b4..e7695b7ae 100644 --- a/spec/android_merge_translators_strings_spec.rb +++ b/spec/android_merge_translators_strings_spec.rb @@ -35,7 +35,7 @@ def amts_run_test(script) test_script = @amtsTestUtils.get_test_from_file(script) @amtsTestUtils.create_test_data(test_script) - Fastlane::Actions::AndroidMergeTranslatorsStringsAction.run(strings_folder: @amtsTestUtils.test_folder_path) + run_described_fastlane_action(strings_folder: @amtsTestUtils.test_folder_path) expect(@amtsTestUtils.read_result_data(test_script)).to eq(test_script['result']['content']) end diff --git a/spec/ios_lint_localizations_spec.rb b/spec/ios_lint_localizations_spec.rb index 4e6ac3888..036acff29 100644 --- a/spec/ios_lint_localizations_spec.rb +++ b/spec/ios_lint_localizations_spec.rb @@ -23,7 +23,7 @@ expect(FileUtils).to receive(:cp_r) expect_shell_command("#{install_dir}/bin/swiftgen", 'config', 'run', '--config', anything) - Fastlane::Actions::IosLintLocalizationsAction.run( + run_described_fastlane_action( install_path: install_dir, input_dir: empty_dataset, base_lang: 'en' @@ -43,7 +43,7 @@ # Second run: ensure we only run SwiftGen directly, without a call to curl nor unzip beforehand expect_shell_command("#{install_dir}/bin/swiftgen", 'config', 'run', '--config', anything) - Fastlane::Actions::IosLintLocalizationsAction.run( + run_described_fastlane_action( install_path: install_dir, input_dir: empty_dataset, base_lang: 'en' @@ -103,7 +103,7 @@ def run_l10n_linter_test(data_file) # remove this after the test ends, so that further executions of the test run faster. # Only the first execution of the tests might take longer if it needs to install SwiftGen first to be able to run the tests. install_dir = "vendor/swiftgen/#{Fastlane::Helper::Ios::L10nLinterHelper::SWIFTGEN_VERSION}" - result = Fastlane::Actions::IosLintLocalizationsAction.run( + result = run_described_fastlane_action( install_path: install_dir, input_dir: @test_data_dir, base_lang: 'en' diff --git a/spec/ios_merge_translators_strings_spec.rb b/spec/ios_merge_translators_strings_spec.rb index f20eebfef..4ae829f8f 100644 --- a/spec/ios_merge_translators_strings_spec.rb +++ b/spec/ios_merge_translators_strings_spec.rb @@ -35,7 +35,7 @@ def imts_run_test(script) test_script = @imtsTestUtils.get_test_from_file(script) @imtsTestUtils.create_test_data(test_script) - Fastlane::Actions::IosMergeTranslatorsStringsAction.run(strings_folder: @imtsTestUtils.test_folder_path) + run_described_fastlane_action(strings_folder: @imtsTestUtils.test_folder_path) expect(@imtsTestUtils.read_result_data(test_script)).to eq(test_script['result']['content']) end From 8e96443706645ca2f41a55890b171f75f4396b57 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Tue, 1 Feb 2022 19:18:27 +0100 Subject: [PATCH 09/10] Improved run_described_fastlane_action to reduce logs during tests cc @mokagio --- spec/spec_helper.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index a2198e746..8bc4ad62d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -70,17 +70,19 @@ def expect_shell_command(*command, exitstatus: 0, output: '') # If the `described_class` of a spec is a `Fastlane::Action` subclass, it runs # it with the given parameters. # -def run_described_fastlane_action(parameters, lane_name = 'test') - expect(described_class).to be < Fastlane::Action, "Only call `#{__callee__}` from a spec describing a `Fastlane::Action` subclass." +def run_described_fastlane_action(parameters) + raise "Only call `#{__callee__}` from a spec describing a `Fastlane::Action` subclass." unless Fastlane::Actions.is_class_action?(described_class) + # Avoid logging messages about deprecated actions while running tests on them + allow(Fastlane::Actions).to receive(:is_deprecated?).and_return(false) lane = <<~LANE - lane :#{lane_name} do + lane :test do #{described_class.action_name}( #{parameters.inspect} ) end LANE - Fastlane::FastFile.new.parse(lane).runner.execute(lane_name.to_sym) + Fastlane::FastFile.new.parse(lane).runner.execute(:test) end # Executes the given block within an ad hoc temporary directory. From 8448e02b4b474858d0ffb78067631115619e7235 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Tue, 1 Feb 2022 21:09:48 +0100 Subject: [PATCH 10/10] Add `CHANGELOG.md` entry for #330 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86f7618b9..9d8e9685c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ _None_ * Ensure that the `gem push` step only runs on CI if lint, test and danger steps passed before it. [#325] * Rename internal `Ios::L10nHelper` to `Ios::L10nLinterHelper`. [#328] +* Provide new `run_described_fastlane_action` to run Fastlane actions more thoroughly in unit tests [#330] ## 2.3.0