Improve SSHKit::Host comparison

 * #equal? should test for object identity, not whether the contents
   are equal, as per the Ruby documentation.
 * #eql? and #== should check the contents directly instead of comparing
   the hash. This prevents false positives.

Author: FooBarWidget

lib/sshkit/host.rb

@@ -5,6 +5,7 @@ module SSHKit
   UnparsableHostStringError = Class.new(SSHKit::StandardError)
 
   class Host
+    DEFAULT_SSH_PORT = 22
 
     attr_accessor :password, :hostname, :port, :user, :ssh_options
 
@@ -60,10 +61,12 @@ def username
     end
 
     def eql?(other_host)
-      other_host.hash == hash
+      other_host &&
+        user == other_host.user &&
+        hostname == other_host.hostname &&
+        port_with_default == other_host.send(:port_with_default)
     end
     alias :== :eql?
-    alias :equal? :eql?
 
     def to_s
       hostname
@@ -84,6 +87,12 @@ def properties
       @properties ||= OpenStruct.new
     end
 
+    private
+
+    def port_with_default
+      port || DEFAULT_SSH_PORT
+    end
+
   end
 
   # @private

test/unit/test_host.rb

@@ -58,7 +58,23 @@ def test_assert_hosts_hash_equally
     def test_assert_hosts_compare_equal
       assert Host.new('example.com') == Host.new('example.com')
       assert Host.new('example.com').eql? Host.new('example.com')
-      assert Host.new('example.com').equal? Host.new('example.com')
+
+      assert Host.new('example.com:22') == Host.new('example.com')
+      assert Host.new('example.com:22').eql? Host.new('example.com')
+
+      assert Host.new('example.com:22') != Host.new('example.com:23')
+      assert !Host.new('example.com:22').eql?(Host.new('example.com:23'))
+
+      assert Host.new('[email protected]') == Host.new('[email protected]')
+      assert Host.new('[email protected]').eql? Host.new('[email protected]')
+
+      assert Host.new('[email protected]') != Host.new('[email protected]')
+      assert !Host.new('[email protected]').eql?(Host.new('[email protected]'))
+
+      a = Host.new('example.com')
+      b = Host.new('example.com')
+      assert a.equal? a
+      assert !a.equal?(b)
     end
 
     def test_arbitrary_host_properties