Order interactions within the contract by provider state.

[amalkov]

In order to avoid unnecessary contract file changes.

Generally RSpec does not execute tests in alphabetical order, it means that if you run the test without any changes several times, it creates pact contract with interactions in random order too. As a result you need to be careful to understand do you have any changes in pact contract or just commit changes every time it changes the contract.

[bethesque]

I have this funny feeling that there's some other code somewhere that does this. I'll try and remember.

[bethesque]

I'd like this to be configurable, because (and I know this is bad and I don't encourage it) there are people who need the tests to be executed in a certain order when they replay them. I remember having conversations about it.

[bethesque]

Also! This is the order that the pact broker displays the interactions in - description, status, provider state. It would be good to keep this consistent.

module Pact
  module Doc
    class SortInteractions

      def self.call interactions
        interactions.sort{|a, b| sortable_id(a) <=> sortable_id(b)}
      end

      private

      def self.sortable_id interaction
        "#{interaction.description.downcase} #{interaction.response.status} #{(interaction.provider_state || '').downcase}"
      end

    end
  end
end

It means that they're sorted thusly: "a request for an alligator - given an alligator exists - 200" then "a request for an alligator - given an alligator does not exist - 400" "a request for something else...".

[bethesque]

Can't find the code that I thought might be there. Happy for this to be accepted as long as it is configurable and we don't break semantic versioning.

[amalkov]

Thanks for commenting. I agree, order by "description, status, provider state" sounds better. What do you mean by configurable? I think now anyone can set any order by changing the description...

[bethesque]

Like the pact_file_write_mode

https://github.com/realestate-com-au/pact/blob/master/documentation/configuration.md#pactfile_write_mode

https://github.com/realestate-com-au/pact/blob/master/lib/pact/consumer/configuration/configuration_extensions.rb#L60

Perhaps pact_file_write_order with options chronological (default) and fixed?

[amalkov]

Just to confirm, do you really want to make it configurable? Usually it is a good practice to develop something when you have real needs and exact requirement, otherwise you will develop it and nobody gonna use it in the future...

Current changes just fixing an issues with inconsistency of the pact contract after each run.

[bethesque]

Good practice is also to not change existing behaviour within a major version. I know people who rely on the order of the interaction replay being the same as the order in which they were originally executed (even though this is bad practice and I don't encourage it!). If we change the order they are stored in within the pact file, we change the order they will be replayed in.

[amalkov]

Sorry, I agree with you, but wasn't clear enough. My point is that it should be randomised when replaying contract, not when we are building it. In this case by default:

1. build a contract ordered by "description, status, provider state" - consistency in the generated pact.json file.
2. randomise interactions on replay

And completely different question: Do we want to have a configuration randomised/alphabetical contract replay?

[bethesque]

I absolutely agree with your 2 points. That's the way it should have been done. However, it was not, and now people rely on the current behaviour, and it is not good practice to change it without making a major version change. So, in the meantime, I'd like to make it backwards compatible, with a configuration option to set the order of the interactions in the pact file.

Configurable randomised replay would be great. There is currently no option for this. If you felt like submitting that as a PR, I'd be happy for it to be accepted.

[amalkov]

I've created a pull request on Pact gem. In order to pass pact changes we need to merge this pull request first

[bethesque]

Thanks for your work Alex. Can you explain to me how this maintains backwards compatibility for people who need to replay the interactions in the order in which they were executed?

[amalkov]

By default Rspec running tests in a random order, it means that by default we are saving interactions within a consumer contract in random order and as a result replaying interactions in a random order as well - no changes for people who upgraded pact gem to version "1.10.0".

For those, who is running rspec tests in a specific order - they might need to modify description or provider state in order to record pact contract in specific order and replay it later on in the recorder order.

[bethesque]

For those, who is running rspec tests in a specific order - they might need to modify description or provider state in order to record pact contract in specific order and replay it later on in the recorder order.

Alex, please maintain the current behaviour by default until we can make a breaking change with a major version. Once that is done, I will very happily merge the PR, thanks for your work.

[amalkov]

Beth, honestly, I don't understand what can be done more at this point. The main reason of this pull request and pull request on the pact gem is to change the current behaviour in order to make pact contract consistent but replay - randomised or in order defined by the contract.

[bethesque]

Please add a configuration option pact_file_write_order, with options chronological (default) and alphabetical.

  def sorted_interactions
    return consumer_contract.interactions if pact_file_write_order == :chronological
    consumer_contract.interactions.sort{|a, b| sortable_id(a) <=> sortable_id(b)}
  end

This will maintain the current behaviour by default, but allow the order of the interactions to also be fixed as you desire.

[bethesque]

The changes to the other project are looking good. Let me know when these are done because I don't get a notification when a commit happens, so I won't know when it's ready to merge.