ifesdjeen commented on PR #4373: URL: https://github.com/apache/cassandra/pull/4373#issuecomment-3307985354
I did not yet have a chance to fully review the patch, but since this patch mostly ensures that Accord tables are lazy, I think the amount of testing here is more or less appropriate. I would personally not add extensive tests to, say, test whether or not we are seeing all coordinations, etc, since this will likely just lead to tightly coupling observability with internals (which we unfortunately have in too many places alreadyU I thought we were talking about _remote_ vtables with @clohfink in MN, so I briefly looked at the other patch, too. And from what I could see, the tables that are remote, we do have test coverage. I think testing for _laziness_ itself is probably also a bit excessive, but it also does not seem that it was requested here, so just mentioning this for completeness. tl;dr it does seem that we have enough tests to verify that tables that are lazy, which are AccordDebugKeyspaceTables are working. Moreover, these tables are hidden behind a feature flag that disables them by default (even two feature flags, one is accord, and second one accord debug tables themselves). That said, if we want to have exhaustive coverage for both lazy and distributed tables _machinery_, I think we can also do that: we could generate different random virtual tables, populate them with different test data, and make sure same thing comes out from both ways. Which, I think, should probably have been done when virtual tables first introduced. But I would definitely not enforce it on either of these patches. Could be a fun introductory ticket to Cassandra. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
