Merge pull request #7014 from dnkoutso/filter_test_only_pod_targets

Use the resolver to identify which pod targets are test only

CHANGELOG.md

@@ -34,6 +34,11 @@ To install release candidates run `[sudo] gem install cocoapods --pre`
   [Dimitris Koutsogiorgas](https://github.com/dnkoutso)
   [#7019](https://github.com/CocoaPods/CocoaPods/pull/7019)
 
+* Use the resolver to identify which pod targets are test only  
+  [Dimitris Koutsogiorgas](https://github.com/dnkoutso)
+  [Justin Martin](https://github.com/justinseanmartin)
+  [#7014](https://github.com/CocoaPods/CocoaPods/pull/7014)
+
 * Perform code signing on xctest bundles in the Pods project generated by a test spec  
   [Justin Martin](https://github.com/justinseanmartin)
   [#7013](https://github.com/CocoaPods/CocoaPods/pull/7013)

lib/cocoapods/resolver.rb

@@ -12,6 +12,39 @@ module Pod
   # by target for a given Podfile.
   #
   class Resolver
+    # A small container that wraps a resolved specification for a given target definition. Additional metadata
+    # is included here such as if the specification is only used by tests.
+    #
+    class ResolverSpecification
+      # @return [Specification] the specification that was resolved
+      #
+      attr_reader :spec
+
+      # @return [Bool] whether this resolved specification is only used by tests.
+      #
+      attr_reader :used_by_tests_only
+      alias used_by_tests_only? used_by_tests_only
+
+      def initialize(spec, used_by_tests_only)
+        @spec = spec
+        @used_by_tests_only = used_by_tests_only
+      end
+
+      def name
+        spec.name
+      end
+
+      def root
+        spec.root
+      end
+
+      def ==(other)
+        self.class == other &&
+          spec == other.spec &&
+          used_by_tests_only == other.test_only
+      end
+    end
+
     include Pod::Installer::InstallationOptions::Mixin
 
     delegate_installation_options { podfile }
@@ -64,8 +97,8 @@ module Pod
 
     # Identifies the specifications that should be installed.
     #
-    # @return [Hash{TargetDefinition => Array<Specification>}] specs_by_target
-    #         the specifications that need to be installed grouped by target
+    # @return [Hash{TargetDefinition => Array<ResolverSpecification>}] resolver_specs_by_target
+    #         the resolved specifications that need to be installed grouped by target
     #         definition.
     #
     def resolve
@@ -75,28 +108,28 @@ module Pod
         end
       end
       @activated = Molinillo::Resolver.new(self, self).resolve(dependencies, locked_dependencies)
-      specs_by_target
+      resolver_specs_by_target
     rescue Molinillo::ResolverError => e
       handle_resolver_error(e)
     end
 
-    # @return [Hash{Podfile::TargetDefinition => Array<Specification>}]
+    # @return [Hash{Podfile::TargetDefinition => Array<ResolverSpecification>}]
     #         returns the resolved specifications grouped by target.
     #
     # @note   The returned specifications can be subspecs.
     #
-    def specs_by_target
-      @specs_by_target ||= {}.tap do |specs_by_target|
+    def resolver_specs_by_target
+      @resolver_specs_by_target ||= {}.tap do |resolver_specs_by_target|
         podfile.target_definition_list.each do |target|
           dependencies = {}
           specs = target.dependencies.map(&:name).flat_map do |name|
             node = @activated.vertex_named(name)
-            valid_dependencies_for_target_from_node(target, dependencies, node) << node
+            (valid_dependencies_for_target_from_node(target, dependencies, node) << node).map { |s| [s, node.payload.test_specification?] }
           end
 
-          specs_by_target[target] = specs.
-            map(&:payload).
-            uniq.
+          resolver_specs_by_target[target] = specs.
+            group_by(&:first).
+            map { |vertex, spec_test_only_tuples| ResolverSpecification.new(vertex.payload, spec_test_only_tuples.map { |tuple| tuple[1] }.all?) }.
             sort_by(&:name)
         end
       end
@@ -517,7 +550,6 @@ module Pod
 
         dependency_nodes + dependency_nodes.flat_map do |item|
           node_result = valid_dependencies_for_target_from_node(target, dependencies, item)
-
           node_result
         end
       end

spec/unit/installer/analyzer_spec.rb

@@ -672,84 +672,30 @@ module Pod
 
       #-------------------------------------------------------------------------#
 
-      it 'handles test only pod targets' do
-        pod_target_one = stub(:name => 'Pod1', :dependent_targets => [], :test_dependent_targets => [])
-        pod_target_two = stub(:name => 'Pod2', :dependent_targets => [], :test_dependent_targets => [])
-        pod_target_three = stub(:name => 'Pod3', :dependent_targets => [pod_target_one, pod_target_two], :test_dependent_targets => [])
-        pod_target_four = stub(:name => 'Pod4', :dependent_targets => [], :test_dependent_targets => [pod_target_three])
-
-        all_pod_targets = [pod_target_one, pod_target_two, pod_target_three, pod_target_four]
-        @analyzer.send(:pod_target_test_only?, pod_target_one, all_pod_targets).should.be.false
-        @analyzer.send(:pod_target_test_only?, pod_target_two, all_pod_targets).should.be.false
-        @analyzer.send(:pod_target_test_only?, pod_target_three, all_pod_targets).should.be.true
-        @analyzer.send(:pod_target_test_only?, pod_target_four, all_pod_targets).should.be.false
-      end
-
-      it 'handles test only pod targets that depend on themselves as tests' do
-        pod_target_one = stub(:name => 'Pod1', :dependent_targets => [])
-        pod_target_one.stubs(:test_dependent_targets => [pod_target_one])
-        all_pod_targets = [pod_target_one]
-        @analyzer.send(:pod_target_test_only?, pod_target_one, all_pod_targets).should.be.false
-      end
-
-      it 'handles test only pod targets that depend on themselves as tests but are also dependent as sources' do
-        pod_target_one = stub(:name => 'Pod1', :dependent_targets => [])
-        pod_target_one.stubs(:test_dependent_targets => [pod_target_one])
-        pod_target_two = stub(:name => 'Pod2', :dependent_targets => [pod_target_one], :test_dependent_targets => [])
-        all_pod_targets = [pod_target_one, pod_target_two]
-        @analyzer.send(:pod_target_test_only?, pod_target_one, all_pod_targets).should.be.false
-      end
-
-      it 'includes pod target when declared in the target definition and is not test only' do
-        target_definition = stub
-        pod_target = stub(:name => 'Pod1', :dependent_targets => [], :test_dependent_targets => [], :target_definitions => [target_definition])
-        all_pod_targets = [pod_target]
-        @analyzer.stubs(:pod_target_test_only?).with(pod_target, all_pod_targets).returns(false)
-        @analyzer.send(:filter_pod_targets_for_target_definition, all_pod_targets, target_definition).should == [pod_target]
-      end
-
-      it 'includes pod target when declared in the pod target definition but has a test dependency on itself' do
+      it 'does include pod target if any spec is not used by tests only and is part of target definition' do
+        spec1 = Resolver::ResolverSpecification.new(stub, false)
+        spec2 = Resolver::ResolverSpecification.new(stub, true)
         target_definition = stub
-        pod_target = stub(:name => 'Pod1', :dependent_targets => [], :target_definitions => [target_definition])
-        pod_target.stubs(:test_dependent_targets => [pod_target])
-        all_pod_targets = [pod_target]
-        @analyzer.send(:filter_pod_targets_for_target_definition, all_pod_targets, target_definition).should == [pod_target]
+        pod_target = stub(:name => 'Pod1', :target_definitions => [target_definition], :specs => [spec1.spec, spec2.spec])
+        resolver_specs_by_target = { target_definition => [spec1, spec2] }
+        @analyzer.send(:filter_pod_targets_for_target_definition, target_definition, [pod_target], resolver_specs_by_target).should == [pod_target]
       end
 
-      it 'does not include pod target if declared within pod target definition and is a test only target' do
+      it 'does not include pod target if its used by tests only' do
+        spec1 = Resolver::ResolverSpecification.new(stub, true)
+        spec2 = Resolver::ResolverSpecification.new(stub, true)
         target_definition = stub
-        pod_target = stub(:name => 'Pod1', :dependent_targets => [], :test_dependent_targets => [], :target_definitions => [target_definition])
-        all_pod_targets = [pod_target]
-        @analyzer.stubs(:pod_target_test_only?).with(pod_target, all_pod_targets).returns(true)
-        @analyzer.send(:filter_pod_targets_for_target_definition, all_pod_targets, target_definition).should.be.empty
+        pod_target = stub(:name => 'Pod1', :target_definitions => [target_definition], :specs => [spec1.spec, spec2.spec])
+        resolver_specs_by_target = { target_definition => [spec1, spec2] }
+        @analyzer.send(:filter_pod_targets_for_target_definition, target_definition, [pod_target], resolver_specs_by_target).should.be.empty
       end
 
-      it 'does not include pod target if not within target definition' do
+      it 'does not include pod target if its not part of the target definition' do
+        spec = Resolver::ResolverSpecification.new(stub, false)
         target_definition = stub
-        pod_target = stub(:name => 'Pod1', :dependent_targets => [], :test_dependent_targets => [], :target_definitions => [])
-        all_pod_targets = [pod_target]
-        @analyzer.send(:filter_pod_targets_for_target_definition, all_pod_targets, target_definition).should.be.empty
-      end
-
-      it 'handles complicated scenario of pod target dependencies' do
-        target_definition_one = stub
-        target_definition_two = stub
-        pod_target_one = stub(:name => 'Pod1', :dependent_targets => [], :test_dependent_targets => [], :target_definitions => [target_definition_one, target_definition_two])
-        pod_target_two = stub(:name => 'Pod2', :dependent_targets => [], :test_dependent_targets => [], :target_definitions => [target_definition_one])
-        pod_target_three = stub(:name => 'Pod3', :dependent_targets => [], :target_definitions => [target_definition_two])
-        pod_target_three.stubs(:test_dependent_targets => [pod_target_three])
-        pod_target_four = stub(:name => 'Pod4', :dependent_targets => [pod_target_one], :test_dependent_targets => [], :target_definitions => [target_definition_one])
-        pod_target_five = stub(:name => 'Pod5', :dependent_targets => [pod_target_one], :test_dependent_targets => [pod_target_three], :target_definitions => [target_definition_two])
-        all_pod_targets = [pod_target_one, pod_target_two, pod_target_three, pod_target_four, pod_target_five]
-        @analyzer.send(:filter_pod_targets_for_target_definition, all_pod_targets, target_definition_one).should == [
-          pod_target_one,
-          pod_target_two,
-          pod_target_four,
-        ]
-        @analyzer.send(:filter_pod_targets_for_target_definition, all_pod_targets, target_definition_two).should == [
-          pod_target_one,
-          pod_target_five,
-        ]
+        pod_target = stub(:name => 'Pod1', :target_definitions => [], :specs => [spec.spec])
+        resolver_specs_by_target = { target_definition => [spec] }
+        @analyzer.send(:filter_pod_targets_for_target_definition, target_definition, [pod_target], resolver_specs_by_target).should.be.empty
       end
 
       describe 'extension targets' do