Sarama: SyncProducer.SendMessages doesn't work properly in tests

[luismfonseca]

Versions

Please specify real version numbers or git SHAs, not just "Latest" since that changes fairly regularly.
Sarama Version: 02452b3987ac0e7b93702149e96d4f9b2e6521ec
Kafka Version: not applicable
Go Version: go version go1.7.3

Problem Description

The MR #677 brought the SendMessages function and its mock counterpart. Now the test didn't really test assertions in the form of:

        producer.ExpectSendMessageAndFail(sarama.ErrOutOfBrokers)

So somewhere in the history of this file, it stopped working.
If I change the mock to use the sister function SendMessage, my tests work:

func (sp *SyncProducer) SendMessages(msgs []*sarama.ProducerMessage) error {
    for _, msg := range msgs {
        _, _, err := sp.SendMessage(msg)
        if err != nil {
            return err
        }
    }
    return nil
}

But I don't know if that would be the correct fix. Another way to fix this just by editing this line:

        expectations := sp.expectations[0 : len(msgs)-1]

to

        expectations := sp.expectations[0 : len(msgs)]

If there's 1 message, a slice from 0 to 1 - 1 is going to be an empty slice.
But this fix does not make the SendMessages actually use the expectation.CheckFunction.

I can write a minimal test for this if this description was too confusing, and a merge request if someone tells me the preferred way to fix this.

[luismfonseca]

Can I make a merge request changing:
expectations := sp.expectations[0 : len(msgs)-1]
to
expectations := sp.expectations[0 : len(msgs)]
?

[eapache]

Nice catch on both issues, I'd be happy to take pull requests to fix them.

I think your suggestion to just loop and call SendMessage is the best fix since it will then automatically pick up any future changes.