Add support for test target creation in the pods project generator #6703

dnkoutso · 2017-05-10T22:05:12Z

No description provided.

dnkoutso · 2017-05-10T22:05:33Z

Gemfile

@@ -25,7 +25,7 @@ gem 'json', :git => 'https://github.com/segiddins/json.git', :branch => 'seg-1.7



will be reverted.

dnkoutso · 2017-05-10T22:05:39Z

Gemfile.lock

@@ -6,16 +6,6 @@ GIT
    claide (1.0.1)


will be reverted.

dnkoutso · 2017-05-10T22:05:54Z

lib/cocoapods/generator/xcconfig/aggregate_xcconfig.rb

@@ -176,40 +177,6 @@ def generate_settings_to_import_pod_targets
          end
        end

-        # Add custom build settings and required build settings to link to


Those moved to xcconfig_helper no changes here and specs pass for this class.

dnkoutso · 2017-05-10T23:07:59Z

lib/cocoapods/generator/xcconfig/pod_xcconfig.rb

@@ -66,6 +70,11 @@ def generate
          end
          XCConfigHelper.add_target_specific_settings(target, @xcconfig)
          @xcconfig.merge! XCConfigHelper.settings_for_dependent_targets(target, target.recursive_dependent_targets)
+          if @test_xcconfig


For test xcconfigs we ensure that linker flags are added for their dependencies.

dnkoutso · 2017-05-10T23:08:11Z

lib/cocoapods/generator/xcconfig/xcconfig_helper.rb

          ld_flags = ''
-          ld_flags << '-ObjC' if includes_static_libraries
+          ld_flags << '-ObjC' if include_objc_flag


just rename of the flag here no real changes.

dnkoutso · 2017-05-10T23:08:42Z

lib/cocoapods/generator/xcconfig/xcconfig_helper.rb

@@ -296,6 +298,55 @@ def self.settings_for_dependent_targets(target, dependent_targets)
          build_settings
        end

+        # Add custom build settings and required build settings to link to


copied pasted from aggregate_xcconfig.rb, almost zero changes except passing parameters.

dnkoutso · 2017-05-10T23:08:58Z

lib/cocoapods/installer/xcode/pods_project_generator.rb

@@ -86,6 +86,11 @@ def share_development_pod_schemes
          development_pod_targets.select(&:should_build?).each do |pod_target|
            next unless share_scheme_for_development_pod?(pod_target.pod_name)
            Xcodeproj::XCScheme.share_scheme(project.path, pod_target.label)
+            if pod_target.contains_test_specifications?


Shares the xcscheme for tests :D

dnkoutso · 2017-05-10T23:09:06Z

lib/cocoapods/installer/xcode/pods_project_generator.rb

              end
            end
          end
        end

+        # TODO


will take care of this

dnkoutso · 2017-05-10T23:09:23Z

lib/cocoapods/installer/xcode/pods_project_generator/pod_target_installer.rb

              if target.requires_frameworks?
                create_info_plist_file
                create_module_map
                create_umbrella_header do |generator|
+                  file_accessors = target.file_accessors.reject { |f| f.spec.test_specification? }


This prevents adding test specification headers as part of the generated umbrella header

you may want to gate the reject on contains_test_specification - might be expensive to do this for larger projects that do not have test specs

👍 nice thanks for the tip will do

@dantoml done, also added a test case to ensure test header files never make it into umbrella header :)

dnkoutso · 2017-05-10T23:09:33Z

lib/cocoapods/installer/xcode/pods_project_generator/pod_target_installer.rb

+            end
+          end
+
+          # TODO: doc


dnkoutso · 2017-05-10T23:09:46Z

lib/cocoapods/target/pod_target.rb

@@ -234,7 +267,7 @@ def include_in_build_config?(target_definition, configuration_name)

      if whitelists.empty?
        @build_config_cache[key] = true
-        return true


just warning fix

dnkoutso · 2017-05-10T23:09:49Z

lib/cocoapods/target/pod_target.rb

@@ -258,7 +291,7 @@ def inhibit_warnings?

      if whitelists.empty?
        @inhibit_warnings = false
-        return false


just warning fix

dnkoutso · 2017-05-10T23:09:53Z

lib/cocoapods/target/pod_target.rb

@@ -268,7 +301,7 @@ def inhibit_warnings?
          'settings to inhibit warnings. CocoaPods does not currently ' \
          'support different settings and will fall back to your preference ' \
          'set in the root target definition.'
-        return podfile.root_target_definitions.first.inhibits_warnings_for_pod?(root_spec.name)
+        podfile.root_target_definitions.first.inhibits_warnings_for_pod?(root_spec.name)


just warning fix

dnkoutso · 2017-05-10T23:10:13Z

spec/fixtures/coconut-lib/CoconutLib.podspec

@@ -0,0 +1,20 @@
+Pod::Spec.new do |s|


New library that helps with test_spec support

dnkoutso · 2017-05-10T23:11:17Z

@benasher44, @dantoml, @orta (and maybe @segiddins?) would love for you to review this PR at your time.

I need to add a few more specs but overall its 95% complete.

dnkoutso · 2017-05-11T02:27:44Z

lib/cocoapods/installer/xcode/pods_project_generator.rb

@@ -203,16 +208,10 @@ def set_target_dependencies
              aggregate_target.native_target.add_dependency(pod_target.native_target)
              configure_app_extension_api_only_for_target(pod_target) if is_app_extension

-              pod_target.dependent_targets.each do |pod_dependency_target|


method extracted. No logical changes. Helps support wiring up test dependencies to the native targets

orta · 2017-05-11T08:25:21Z

@dnkoutso any chance you can upload a zip of an example project with this?

dnkoutso · 2017-05-11T14:49:32Z

@orta will upload one today

dnkoutso · 2017-05-11T18:32:55Z

@orta there you go.

Please keep in mind I am still using a Podfile to consume the test spec since the validator work is not done.

You will also need to ensure you setup cocoapods to point to my repo and branch so the integration works.

It's pretty awesome to see it in action :)

SampleProject.zip

dnkoutso · 2017-05-11T18:54:54Z

Added a few more specs!

dnkoutso · 2017-05-11T20:39:01Z

Should go green!

dnkoutso · 2017-05-11T23:09:56Z

Green!

endocrimes

great work so far - this is actually looking to be pretty solid.

We'll definitely need guides for this - and potentially a separate project generation command for working on and testing pods outside of a host application project.

endocrimes · 2017-05-14T20:34:08Z

lib/cocoapods/installer/xcode/pods_project_generator/pod_target_installer.rb

@@ -400,7 +489,7 @@ def header_mappings_dir
                                   end
          end

-          def add_header(build_file, public_headers, private_headers)
+          def add_header(build_file, public_headers, private_headers, native_target)


this added param is not used

It is, the method uses native_target which is a property but now the parameter takes over so no real method changes were required.

ahhh - lol shadowing

endocrimes · 2017-05-14T20:36:01Z

lib/cocoapods/installer/xcode/pods_project_generator/pod_target_installer.rb

              if target.requires_frameworks?
                create_info_plist_file
                create_module_map
                create_umbrella_header do |generator|
+                  file_accessors = target.file_accessors.reject { |f| f.spec.test_specification? }


you may want to gate the reject on contains_test_specification - might be expensive to do this for larger projects that do not have test specs

dnkoutso · 2017-05-15T06:27:44Z

All set.

orta · 2017-05-15T10:16:04Z

I wonder if we can do something about making cmd + u work for the project. Right now, tests only work inside the actual test target.

I wonder if we can make it work both for the library:

So for this setup:

Ideally cmd + u on Pods-SampleProject, and SampleLibrary would also work. It would run any tests for any pod-targets that are linked to the "cocoapods target"

endocrimes · 2017-05-15T10:17:38Z

@orta Yeah it would be pretty nice if we added the testable to the main scheme

dnkoutso · 2017-05-15T14:50:30Z

Yes! So CMD+U works today on the test scheme but only if you decide to have share_development_pod_schemes enabled.

Second, Xcodeproj has a method called recreate_user_schemes so I tried to edit the xcconfigs but there we being "nuked" again and re-written by this method.

I think we should expose a block in the Xcodeproj method and allow us to perform various customization on the schemes before we save them, so we don't have to do a post-process step and save them twice which can be slow for multiple files.

Is this a blocker in this PR?

orta · 2017-05-15T15:23:18Z

IMO, ideally that gets looked at before the RC releases, but I don't consider it a blocker on this PR which is "get it in" vs "make it perfect"

dnkoutso · 2017-05-15T15:27:32Z

With your blessing we can get this merged and start a bit the process of polishing it!

I got an upcoming branch with validator changes for running tests during lint

endocrimes

lets get this on master and we can polish it as we go.

dnkoutso · 2017-05-15T15:51:11Z

<3!

segiddins · 2017-05-15T17:26:43Z

🍾 well done, @dnkoutso !

dnkoutso commented May 10, 2017

View reviewed changes

Gemfile.lock Outdated

@@ -6,16 +6,6 @@ GIT

claide (1.0.1)

Copy link

Contributor Author

dnkoutso May 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will be reverted.

dnkoutso commented May 10, 2017

View reviewed changes

lib/cocoapods/installer/xcode/pods_project_generator.rb Outdated

end

end

end

end

# TODO

Copy link

Contributor Author

dnkoutso May 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will take care of this

dnkoutso commented May 10, 2017

View reviewed changes

lib/cocoapods/installer/xcode/pods_project_generator/pod_target_installer.rb Outdated

end

end

# TODO: doc

Copy link

Contributor Author

dnkoutso May 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will fix

dnkoutso commented May 10, 2017

View reviewed changes

dnkoutso force-pushed the test_phase_1 branch from f492a49 to 41aa4d7 Compare May 11, 2017 01:51

dnkoutso commented May 11, 2017

View reviewed changes

dnkoutso force-pushed the test_phase_1 branch from 41aa4d7 to 5891046 Compare May 11, 2017 18:54

dnkoutso requested review from mrackwitz, orta, benasher44 and endocrimes and removed request for mrackwitz May 11, 2017 18:55

dnkoutso force-pushed the test_phase_1 branch 3 times, most recently from 1c1f5e9 to 1ddb50e Compare May 11, 2017 20:30

dnkoutso force-pushed the test_phase_1 branch from 1ddb50e to 3a48921 Compare May 11, 2017 20:38

dnkoutso force-pushed the test_phase_1 branch 2 times, most recently from c83133d to aa060c8 Compare May 11, 2017 21:15

endocrimes reviewed May 14, 2017

View reviewed changes

dnkoutso force-pushed the test_phase_1 branch 2 times, most recently from da8af81 to 58cb912 Compare May 15, 2017 06:25

Add support for test target creation in the pods project generator

2753b12

dnkoutso force-pushed the test_phase_1 branch from 58cb912 to 2753b12 Compare May 15, 2017 06:27

endocrimes approved these changes May 15, 2017

View reviewed changes

dnkoutso merged commit 5ae791f into CocoaPods:master May 15, 2017

orta mentioned this pull request May 16, 2017

Create xctest target using cocoapods #6649

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for test target creation in the pods project generator #6703

Add support for test target creation in the pods project generator #6703

		@@ -25,7 +25,7 @@ gem 'json', :git => 'https://github.com/segiddins/json.git', :branch => 'seg-1.7

                             end
                           end
                         end
                       end
+                      # TODO

Add support for test target creation in the pods project generator #6703

Add support for test target creation in the pods project generator #6703

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment