fix undeterministic import order

lib/travis/yml/configs/config/base.rb

@@ -1,5 +1,6 @@
 require 'forwardable'
 require 'travis/yml/helper/obj'
+require 'travis/yml/configs/config/order'
 require 'travis/yml/configs/errors'
 require 'travis/yml/configs/ref'
 
@@ -32,6 +33,10 @@ def config
             @config ||= {}
           end
 
+          def config=(config)
+            @config = config
+          end
+
           def load(&block)
             @on_loaded = block if root?
           end
@@ -51,6 +56,19 @@ def imports
             end
           end
           memoize :imports
+
+          def merge
+            return {} if skip? || errored? || circular? || !matches?
+            order_duplicates if root?
+            imports.map(&:merge).inject(part) do |lft, rgt|
+              Support::Merge.new(lft.to_h, rgt.to_h).apply
+            end
+          end
+
+          def order_duplicates
+            Order.new(self).run
+          end
+
           # Flattening the tree should result in a unique array of configs
           # ordered by the order resulting in walking the tree depth-first.
           # However, we load the tree breadth-first and load times vary.
@@ -87,6 +105,10 @@ def sort(configs)
             end
           end
 
+          def errored?
+            !!@errored
+          end
+
           def circular?
             parents.map(&:to_s).include?(to_s)
           end
@@ -130,6 +152,10 @@ def skip
             @skip = true
           end
 
+          def unskip
+            @skip = false
+          end
+
           def skip?
             !!@skip
           end
@@ -184,10 +210,6 @@ def required?
               !parent&.api? || !travis_yml?
             end
 
-            def errored?
-              !!@errored
-            end
-
             def secure?(obj)
               case obj
               when Hash

lib/travis/yml/configs/config/order.rb

@@ -0,0 +1,38 @@
+module Travis
+  module Yml
+    module Configs
+      module Config
+        class Order < Obj.new(:config)
+          # make sure we keep the last duplicate, no matter in which order
+          # configs have been loaded
+          def run
+            order_duplicates(configs.reverse)
+          end
+
+          def configs
+            @configs ||= flatten(config)
+          end
+
+          def flatten(config)
+            return [] if config.errored? || config.circular? || !config.matches?
+            [config] + config.imports.map { |config| flatten(config) }.flatten
+          end
+
+          def order_duplicates(configs)
+            configs.each.with_index do |lft, ix|
+              next unless lft.skip?
+              rgt = configs[(ix + 1)..-1].detect { |rgt| lft.to_s == rgt.to_s && !rgt.skip? }
+              swap(lft, rgt) if rgt
+            end
+          end
+
+          def swap(lft, rgt)
+            lft.unskip
+            rgt.skip
+            lft.config, rgt.config = rgt.config, nil
+          end
+        end
+      end
+    end
+  end
+end

lib/travis/yml/configs/configs.rb

@@ -91,6 +91,7 @@ def fetch
 
           def merge
             doc = Yml.apply(Yml::Parts::Merge.new(configs).apply.to_h, opts) if opts[:merge_normalized]
+            doc ||= Yml.load([ctx.fetch.config.merge], opts) if opts[:merge_mode_2] || true
             doc ||= Yml.load(configs.map(&:part), opts)
             @config = except(doc.serialize, *DROP)
             msgs.concat(doc.msgs)

lib/travis/yml/configs/model/repos.rb

@@ -32,7 +32,7 @@ def []=(slug, repo)
           end
 
           def fetch(slug, provider)
-            logger.info "Get Repo for #{slug} #{provider}"
+            logger.info "Get Repo for #{slug} #{provider}" unless ENV['ENV'] == 'test'
             Travis::Repo.new(slug, provider).fetch
           end
 

lib/travis/yml/remote_vcs/client.rb

@@ -23,7 +23,7 @@ def http_options
 
           def request(method, name)
             resp = connection.send(method) { |req| yield(req) }
-            logger.info("RemoteVcs response #{resp.inspect}")
+            logger.info("RemoteVcs response #{resp.inspect}") unless ENV['ENV'] == 'test'
             JSON.parse(resp.body)
           end
 

lib/travis/yml/remote_vcs/repository.rb

@@ -5,7 +5,7 @@ module Yml
     module RemoteVcs
       class Repository < Client
         def content(id:, path:, ref:)
-          logger.info("RemoteVcs Repository #{id}, #{path}")
+          logger.info("RemoteVcs Repository #{id}, #{path}") unless ENV['ENV'] == 'test'
           request(:get, __method__) do |req|
             req.url "repos/#{id}/contents/#{path}"
             req.params['ref'] = ref

spec/travis/yml/configs/config/order_spec.rb

@@ -0,0 +1,89 @@
+describe Travis::Yml::Configs::Config::Order, 'keep the last duplicate' do
+  let(:const) do
+    Class.new(Obj.new(:source, config: nil, circular: false, errored: false, matches: true)) do
+      def imports
+        @imports ||= []
+      end
+
+      def skip?
+        !!@skip
+      end
+
+      def skip
+        @skip = true
+      end
+
+      def unskip
+        @skip = false
+      end
+
+      alias circular? circular
+      alias errored? errored
+      alias matches? matches
+
+      def to_s
+        source
+      end
+    end
+  end
+
+  let(:config) { const.new('.travis.yml') }
+
+  describe 'flat' do
+    before do
+      0.upto(3) do |ix|
+        config.imports << const.new('one.yml', ix).tap do |config|
+          config.skip if skips[ix]
+        end
+      end
+      described_class.new(config).run
+    end
+
+    describe 'unskipped 1st position' do
+      let(:skips) { [false, true, true, true] }
+      it { expect(config.imports.map(&:skip?)).to eq [true, true, true, false] }
+    end
+
+    describe 'unskipped 2nd position' do
+      let(:skips) { [true, false, true, true] }
+      it { expect(config.imports.map(&:skip?)).to eq [true, true, true, false] }
+    end
+
+    describe 'unskipped 3rd position' do
+      let(:skips) { [true, true, false, true] }
+      it { expect(config.imports.map(&:skip?)).to eq [true, true, true, false] }
+    end
+
+    describe 'unskipped 4th position' do
+      let(:skips) { [true, true, true, false] }
+      it { expect(config.imports.map(&:skip?)).to eq [true, true, true, false] }
+    end
+  end
+
+  describe 'nested' do
+    let(:nested) { config.imports.map { |config| config.imports }.flatten }
+
+    before do
+      config.imports << const.new('one.yml')
+      config.imports << const.new('two.yml')
+
+      0.upto(1) do |ix|
+        config.imports[ix].imports << const.new('nested.yml').tap do |config|
+          config.skip if skips[ix]
+        end
+      end
+
+      described_class.new(config).run
+    end
+
+    describe 'unskipped 1st position' do
+      let(:skips) { [false, true] }
+      it { expect(nested.map(&:skip?)).to eq [true, false] }
+    end
+
+    describe 'unskipped 2nd position' do
+      let(:skips) { [true, false] }
+      it { expect(nested.map(&:skip?)).to eq [true, false] }
+    end
+  end
+end

spec/travis/yml/configs/import_spec.rb

@@ -399,4 +399,84 @@ def self.imports(sources)
       )
     end
   end
+
+  describe do
+    let(:repo) { { id: 1, github_id: 1, slug: 'owner/repo', token: repo_token, private: true } }
+
+    let(:travis_yml) do
+      <<~yml
+        script:
+        - ./travis_yml
+        import:
+        - source: one.yml
+          mode: #{mode}
+        - source: two.yml
+          mode: #{mode}
+      yml
+    end
+
+    let(:one) do
+      <<~yml
+        script:
+        - ./one
+        import:
+        - source: nested.yml
+          mode: #{mode}
+      yml
+    end
+
+    let(:two) do
+      <<~yml
+        script:
+        - ./two
+        import:
+        - source: nested.yml
+          mode: #{mode}
+      yml
+    end
+
+    let(:nested) do
+      <<~yml
+        script:
+        - ./nested
+      yml
+    end
+
+    before { stub_repo(repo[:slug], internal: true, body: repo) }
+    before { stub_content(repo[:id], '.travis.yml', travis_yml) }
+    before { stub_content(repo[:id], 'one.yml', one) }
+    before { stub_content(repo[:id], 'two.yml', two) }
+    before { stub_content(repo[:id], 'nested.yml', nested) }
+    before { configs.tap(&:load) }
+
+    describe 'deep_merge_append' do
+      let(:mode) { :deep_merge_append }
+
+      it do
+        expect(configs.config).to eq(
+          script: %w(
+            ./nested
+            ./two
+            ./one
+            ./travis_yml
+          )
+        )
+      end
+    end
+
+    describe 'deep_merge_prepend' do
+      let(:mode) { :deep_merge_prepend }
+
+      it do
+        expect(configs.config).to eq(
+          script: %w(
+            ./travis_yml
+            ./one
+            ./two
+            ./nested
+          )
+        )
+      end
+    end
+  end
 end