Question: Current use of filters in requests can result in test code being unable to exercise behaviour

[electrofelix]

A number of the requests support filters that change how libvirt is queried for the various resources: https://github.com/fog/fog-libvirt/blob/7e534d173dabcd7810b126241091ba91f23f9603/lib/fog/libvirt/requests/compute/list_domains.rb#L4-L28

Corresponding Mock: https://github.com/fog/fog-libvirt/blob/7e534d173dabcd7810b126241091ba91f23f9603/lib/fog/libvirt/requests/compute/list_domains.rb#L97-L103

however because the filter is fully processed in the Real object, and the Mock just ignores it, it means that exercising any of the logic to ensure information is being retrieved or handled correctly around the filters, can be quite tricky.

Consequently this appears to result in the need to duplicate the filtering behaviour in the Mock instance that will subsequently need to be kept aligned with the Real instance. e.g. https://github.com/fog/fog-libvirt/blob/7e534d173dabcd7810b126241091ba91f23f9603/lib/fog/libvirt/requests/compute/list_networks.rb#L46-L69

needs to filter on the same attributes as: https://github.com/fog/fog-libvirt/blob/7e534d173dabcd7810b126241091ba91f23f9603/lib/fog/libvirt/requests/compute/list_networks.rb#L4-L15

Is this the intended usage for Mocks vs Real? Or should the specific requests be wrapped with private functions for both the Mock and Real with the filter code being common for both and subsequently calling the private methods? e.g.:

module Fog
  module Libvirt
    class Compute
        def self.list_networks(filter = { })
          data=[]
          if filter.keys.empty?
            (list_networks + list_defined_networks).each do |network_name|
              data << network_to_attributes()
            end
          else
            data = [network_to_attributes(get_network_by_filter(filter))]
          end
          data
        end

        def self.get_network_by_filter(filter = { })
          case filter.keys.first
            when :uuid
              lookup_by_uuid(filter[:uuid])
            when :name
              lookup_by_name(filter[:name])
          end
        end
      end

      class Real
        private
        def list_networks
          client.list_networks
        end

        def list_networks_defined
          client.list_networks_defined
        end

        def lookup_by_name(name)
          client.lookup_network_by_name(network_name)
        end

        def lookup_by_uuid(uuid)
          client.lookup_network_by_uuid(uuid)
        end
      end

...

That way the Mock can implement the four calls list_networks, list_networks_defined, lookup_by_name and lookup_by_uuid, retrieving information from an internal stored array, but it still ensures the main logic is exercised and it is only the client request part itself that ends up needing to be mocked and kept in sync.

Note that I'm not expecting the above to be usable as is, just a way of trying to outline how it's a bit difficult to ensure the filtering code in list_networks in the existing code is correct given that any tests won't exercise it, and it's unclear if this is the intended approach or something that has be caused by the fact that for libvirt the filtering results in different API calls being made.