Test that hosts are equal IFF their hashes are equal

[cshaffer]

• Assert all three comparators work (==, eql?, and equal)

[robd]

I like extracting the h1 and h2 variables for the hosts, but I think the expectation lines are quite problematic for a couple of reasons:

1. IMO it is misleading that first_host and second_host would have the same hash, they should have different hashes because the hosts are different. The test sets up the opposite of what would happen in real life.
2. hash returns a number not a string - returning 'example.com' is misleading

I wonder if the following would appease rubocop:

def test_assert_hosts_compare_equal
  h1 = Host.new('example.com')
  h2 = Host.new('example.com')

  assert h1 == h2
  assert h1.eql? h2
  assert h1.equal? h2
end

[cshaffer]

Hi @robd. I don't know if you saw my note on #275 that explained my approach, but basically I'm stating this test is testing too much, basically testing that the initializer is deterministic and obfuscating the reality of what makes two hosts equal. With the stubs it's purposefully ignorant of the "real life" details and only testing what it's meant to, the definition of equality between hosts.

If you still don't agree I'll be happy to change the test to what you describe. Let me know.

[robd]

Hey - yes I did see your note on 275 - sorry I didn't know where it was best to reply.

I understand what you mean about testing just one thing at a time.

But I don't think it helps to set up things which are purposefully different to what happens in the real app. I think this would be very confusing to someone reading the test who didn't realise you had done this. This test reads like different hosts should return the same string hash which is the domain string, and they should be equal.

So if it's OK with you and rubocop, I would be in favour of removing the hash stubbing lines in this case.

As a side note, I think the implementation of the equals methods probably shouldn't use the hash method anyway since there is a (very slim) chance of collisions, but obviously this is unrelated to the rubocop change.

[robd]

👍

[mattbrictson]

🚢