CASSGO-71 Add way to create HostInfo objects for testing purposes #1892
Conversation
joao-r-reis
force-pushed
the
cassgo71-hostfilter-testable
branch
2 times, most recently
from
5d5f2be to
7a91ba4
Compare
jameshartig
left a comment
jameshartig
left a comment
There was a problem hiding this comment.
Just a few minor comments
host_source.go
Outdated
| // | ||
| // If you're looking for a way to create a HostInfo object with more than just an address and port for | ||
| // testing purposes then you can use NewTestHostInfoFromRow | ||
| func NewHostInfoFromContactPoint(addr net.IP, port int) (*HostInfo, error) { |
There was a problem hiding this comment.
nit: What's the reason behind "ContactPoint"? What about "NewHostInfoFromAddrPort"? Not terribly opposed with contact point though
There was a problem hiding this comment.
I checked and the only time we create host info objects from Addr and Port is during init when we are converting contact points into HostPoints, everywhere else we always use the row from the system table. I'm ok with changing to NewHostInfoFromAddrPort though
| // "native_port": 9042, | ||
| // } | ||
| // host, err := NewTestHostInfoFromRow(row) | ||
| func NewTestHostInfoFromRow(row map[string]interface{}) (*HostInfo, error) { |
There was a problem hiding this comment.
Should we have the same method on *Session too so it could still call AddressTranslator?
There was a problem hiding this comment.
Since this is meant to be used for unit testing implementations of policies and other gocql public interfaces I don't think users will be using it with a session so I don't think it's necessary to involve the address translator. Maybe we can consider it if we get requests for it in the future?
joao-r-reis
force-pushed
the
cassgo71-hostfilter-testable
branch
2 times, most recently
from
fcc8b35 to
ebe9a0d
Compare
It is not possible to test implementations of public gocql interfaces such as HostFilter because it requires creating HostInfo objects (HostInfo has private fields). With this change, it is now possible to create HostInfo objects from system.local / system.peers rows using NewTestHostInfoFromRow(). Patch by João Reis; reviewed by James Hartig for CASSGO-71
joao-r-reis
force-pushed
the
cassgo71-hostfilter-testable
branch
from
ebe9a0d to
8b51c53
Compare
2 participants
Also renamed NewHostInfo to NewHostInfoFromContactPoint, this isn't a breaking change because this function was exported recently and no release has gone out since then.
What is a breaking change is making
host.SetHostIDprivate, I don't see why we would let users modify the host id