[GitHub] [spark] viirya edited a comment on pull request #31219: [SPARK-34148][TEST][SS] Move general StateStore tests to StateStoreSuiteBase

[GitBox <gi...@apache.org>]

viirya edited a comment on pull request #31219:
URL: https://github.com/apache/spark/pull/31219#issuecomment-764837648


   Sorry, but I don't think there are strong points for veto.
   
   1. The community did a lot of refactoring, either main or test code. So the first point sounds not make sense.
   2. Moved tests are tests for general StateStore behavior. Without considering other StateStore, they should be put in a general test suite, not HDFS StateStore-specific tests. Specifically, StateStore is an API  that third-party uses for other StateStore implementation. For now. when writing specific StateStore test suite, it needs to duplicate these general tests, otherwise it lacks of test coverage. I don't think this is a good pattern regarding an API like that.
   3. So I also don't think the second point make sense. There is no test specific to so called internal information. They are all based on the StateStore behavior and it is open API.
   
   If any moved test is not belonging to general StateStore, but needs to put in HDFS-specific StateStore, then it should be moved back. This sounds to make more sense to me.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org