From 1b7127bb052096509de60ee5eb098d669c616f32 Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Fri, 16 Dec 2022 04:47:01 -0800 Subject: [PATCH] Improve Codegen Cleanup (#35642) Summary: This PR adds a safety check in case the Cocoapod build script is not able to clean the build folder. We had evidences where this process failed, and in this way the user has a clear and actionable message to fix its situation. ## Changelog [iOS][Added] - Add message with instructions about what to do if the cleanup of the build folder fails. Pull Request resolved: https://github.com/facebook/react-native/pull/35642 Test Plan: I was not able to reproduce the issue locally. The fix is not destructive, let's see if the amount of issues decreases. Reviewed By: dmytrorykun Differential Revision: D42067939 Pulled By: cipolleschi fbshipit-source-id: 433dbfaec42a1bf460dc9a48051aa51ec6e12d16 --- .../cocoapods/__tests__/codegen_utils-test.rb | 51 ++++++++++++++++++- .../cocoapods/__tests__/test_utils/DirMock.rb | 6 ++- .../__tests__/test_utils/FileUtilsMock.rb | 5 +- scripts/cocoapods/codegen_utils.rb | 13 ++++- 4 files changed, 69 insertions(+), 6 deletions(-) diff --git a/scripts/cocoapods/__tests__/codegen_utils-test.rb b/scripts/cocoapods/__tests__/codegen_utils-test.rb index 35f732efac60ce..be5c2cdf7f8173 100644 --- a/scripts/cocoapods/__tests__/codegen_utils-test.rb +++ b/scripts/cocoapods/__tests__/codegen_utils-test.rb @@ -423,12 +423,59 @@ def testCleanUpCodegenFolder_whenFolderExists_deleteItAndSetCleanupDone CodegenUtils.clean_up_build_folder(@base_path, codegen_dir) # Assert - assert_equal(Dir.exist_invocation_params, [codegen_path]) - assert_equal(Dir.glob_invocation, ["#{codegen_path}/*"]) + assert_equal(Dir.exist_invocation_params, [codegen_path, codegen_path]) + assert_equal(Dir.glob_invocation, ["#{codegen_path}/*", "#{codegen_path}/*"]) assert_equal(FileUtils::FileUtilsStorage.rmrf_invocation_count, 1) assert_equal(FileUtils::FileUtilsStorage.rmrf_paths, [globs]) assert_equal(CodegenUtils.cleanup_done(), true) + end + + # ===================================== # + # Test - Assert Codegen Folder Is Empty # + # ===================================== # + + def test_assertCodegenFolderIsEmpty_whenItDoesNotExists_doesNotAbort + # Arrange + codegen_dir = "build/generated/ios" + codegen_path = "#{@base_path}/#{codegen_dir}" + + # Act + CodegenUtils.assert_codegen_folder_is_empty(@base_path, codegen_dir) + + # Assert + assert_equal(Pod::UI.collected_warns, []) + end + def test_assertCodegenFolderIsEmpty_whenItExistsAndIsEmpty_doesNotAbort + # Arrange + codegen_dir = "build/generated/ios" + codegen_path = "#{@base_path}/#{codegen_dir}" + Dir.mocked_existing_dirs(codegen_path) + Dir.mocked_existing_globs([], "#{codegen_path}/*") + + # Act + CodegenUtils.assert_codegen_folder_is_empty(@base_path, codegen_dir) + + # Assert + assert_equal(Pod::UI.collected_warns, []) + end + + def test_assertCodegenFolderIsEmpty_whenItIsNotEmpty_itAborts + # Arrange + codegen_dir = "build/generated/ios" + codegen_path = "#{@base_path}/#{codegen_dir}" + Dir.mocked_existing_dirs(codegen_path) + Dir.mocked_existing_globs(["#{codegen_path}/MyModuleSpecs/MyModule.mm",], "#{codegen_path}/*") + + # Act + assert_raises() { + CodegenUtils.assert_codegen_folder_is_empty(@base_path, codegen_dir) + } + + # Assert + assert_equal(Pod::UI.collected_warns, [ + "Unable to remove the content of ~/app/ios/build/generated/ios folder. Please run rm -rf ~/app/ios/build/generated/ios and try again." + ]) end private diff --git a/scripts/cocoapods/__tests__/test_utils/DirMock.rb b/scripts/cocoapods/__tests__/test_utils/DirMock.rb index 9c7a91328a76ed..c8c229b05c3b36 100644 --- a/scripts/cocoapods/__tests__/test_utils/DirMock.rb +++ b/scripts/cocoapods/__tests__/test_utils/DirMock.rb @@ -49,7 +49,11 @@ def self.glob_invocation def self.glob(path) @@glob_invocation.push(path) - return @@mocked_existing_globs[path] + return @@mocked_existing_globs[path] != nil ? @@mocked_existing_globs[path] : [] + end + + def self.remove_mocked_paths(path) + @@mocked_existing_globs = @@mocked_existing_globs.select { |k, v| v != path } end def self.set_pwd(pwd) diff --git a/scripts/cocoapods/__tests__/test_utils/FileUtilsMock.rb b/scripts/cocoapods/__tests__/test_utils/FileUtilsMock.rb index 4675fcd60cf8bb..cb2155a5e366ee 100644 --- a/scripts/cocoapods/__tests__/test_utils/FileUtilsMock.rb +++ b/scripts/cocoapods/__tests__/test_utils/FileUtilsMock.rb @@ -3,9 +3,9 @@ # This source code is licensed under the MIT license found in the # LICENSE file in the root directory of this source tree. -module FileUtils - +require_relative './DirMock.rb' +module FileUtils class FileUtilsStorage @@RMRF_INVOCATION_COUNT = 0 @@RMRF_PATHS = [] @@ -35,5 +35,6 @@ def self.reset def self.rm_rf(path) FileUtilsStorage.push_rmrf_path(path) FileUtilsStorage.increase_rmrfi_invocation_count + Dir.remove_mocked_paths(path) end end diff --git a/scripts/cocoapods/codegen_utils.rb b/scripts/cocoapods/codegen_utils.rb index 488098d6b6561d..c4aad591a8e490 100644 --- a/scripts/cocoapods/codegen_utils.rb +++ b/scripts/cocoapods/codegen_utils.rb @@ -314,6 +314,17 @@ def self.clean_up_build_folder(app_path, codegen_dir) return if !Dir.exist?(codegen_path) FileUtils.rm_rf(Dir.glob("#{codegen_path}/*")) - CodegenUtils.set_cleanup_done(true) + CodegenUtils.assert_codegen_folder_is_empty(app_path, codegen_dir) + end + + # Need to split this function from the previous one to be able to test it properly. + def self.assert_codegen_folder_is_empty(app_path, codegen_dir) + # double check that the files have actually been deleted. + # Emit an error message if not. + codegen_path = File.join(app_path, codegen_dir) + if Dir.exist?(codegen_path) && Dir.glob("#{codegen_path}/*").length() != 0 + Pod::UI.warn "Unable to remove the content of #{codegen_path} folder. Please run rm -rf #{codegen_path} and try again." + abort + end end end