Hi Yishun, Thank you for the KIP.
I have a couple of comments: 1. Could you please add an example to the KIP that demonstrates how the mocks should be used in a test? 2. I am wondering, whether the MockKeyValueStore needs to be backed by an actual KeyValueStore (in your KIP InMemoryKeyValueStore). Would it not suffice to provide the mock with the entries that it has to check in case of input operation like put() and with the entries it has to return in case of an output operation like get()? In my opinion, a mock should have as little and as simple code as possible. A unit test should depend as little as possible from productive code that it does not explicitly test. 3. I would be interested in the arguments against using a well-established and well-tested mock framework like EasyMock. If there are good arguments, they should be listed under 'Rejected Alternatives'. 3. What is the purpose of the parameter 'time' in MockStoreFactory? Best, Bruno On Tue, Apr 9, 2019 at 11:29 AM Sophie Blee-Goldman <sop...@confluent.io> wrote: > Hi Yishun, thanks for the KIP! I have a few initial questions/comments: > > 1) It may be useful to capture the iterator results as well (eg with a > MockIterator that wraps the underlying iterator and records the same way > the MockStore wraps/records the underlying store) > > 2) a. Where is the "persistent" variable coming from or being used? It > seems the MockKeyValueStore accepts it in the constructor, but only the > name parameter is passed when constructing a new MockKeyValueStore in > build() ... also, if we extend InMemoryXXXStore shouldn't this always be > false? > b. Is the idea to wrap an in-memory store for each type (key-value, > session, etc)? We don't (yet) offer an in-memory version of the session > store although it is in the works, so this will be possible -- I am more > wondering if it makes sense to decide this for the user or to allow them to > choose between in-memory or rocksDB by setting "persistent" > > 3) I'm wondering if users might want to be able to plug in their own custom > stores as the underlying backend...should we support this as well? WDYT? > > 4) We probably want to make these stores available through the public > test-utils package (maybe not the stores themselves which should be > internal, but should there be some kind of public API that gives access to > them?) > > Cheers, > Sophie > > On Tue, Apr 9, 2019 at 9:19 AM Yishun Guan <gyis...@gmail.com> wrote: > > > Bumping this up again, thanks! > > > > On Fri, Apr 5, 2019, 14:36 Yishun Guan <gyis...@gmail.com> wrote: > > > > > Hi, bumping this up again. Thanks! > > > > > > On Tue, Apr 2, 2019, 13:07 Yishun Guan <gyis...@gmail.com> wrote: > > > > > >> Hi All, > > >> > > >> I like to start a discussion on KIP-448 > > >> (https://cwiki.apache.org/confluence/x/SAeZBg). It is about adding > > >> Mock state stores and relevant components for testing purposes. > > >> > > >> Here is the JIRA: https://issues.apache.org/jira/browse/KAFKA-6460 > > >> > > >> This is a rough KIP draft, review and comment are appreciated. It > > >> seems to be tricky and some requirements and details are still needed > > >> to be discussed. > > >> > > >> Thanks, > > >> Yishun > > >> > > > > > > -- Bruno Cadonna Software Engineer at Confluent
