karafka / wiki

Wiki of the main Karafka repository
https://karafka.io
Other
3 stars 12 forks source link

`ApplicationConsumer` testing #23

Open ojab opened 2 years ago

ojab commented 2 years ago

We have something like

class ApplicationConsumer < Karafka::BaseConsumer
  def self.on_unknown(message)
    …
  end

  def consume
    messages.each { |message| consume_single(message) }
  end

  def consume_single(message)
    return self.class.on_unknown(message) unless validate(message)

    MyApp::Metrics.with_context(**metrics_context(message)) do
      process(message)
    end
  end

with a bunch of code handling unknown messages, errors and adding some context for metrics and error reporting in AppicationConsumer and all other consumers inherited from this one. In specs it's tested (or was tested in karafka-1) like

RSpec.describe ApplicationConsumer do
   let(:consumer) do
      Class.new(described_class) do
        def initialize; end # rubocop:disable Style/RedundantInitialize required to avoid exception with different arity
      end.new
    end
end

and with karafka-2 it fails because karafka-testing implies that consumer has topic set, for example here and here. It could be workarounded by

before
  consumer_group = Karafka::Routing::ConsumerGroup.new('foo')  
  consumer.topic = Karafka::Routing::Topic.new('foo', consumer_group)            
end

but would be good to retain the ability to test topic-less consumers or at least simplify/document how consumer group should be created.

mensfeld commented 2 years ago

So, basically you want to test abstract consumers right?

ojab commented 2 years ago

Yep!

mensfeld commented 2 years ago

Now we could get into an argument whether or not you should test abstract classes or their applications ;)

I will think what to do with this in the upcoming days though.

ojab commented 2 years ago

Overall adding documentation would be fine, I guess, since at least in this particular project it's tested in a single spec and adding consumer.topic is not a big deal.

mensfeld commented 2 years ago

@ojab would you mind expanding the docs? They are a wiki: https://github.com/karafka/wiki/blob/master/Testing.md so you can just PR.

mensfeld commented 2 years ago

Moved to wiki as it is for docs expansion