nick-boss-tech commented on PR #5267: URL: https://github.com/apache/solr/pull/5267#issuecomment-6547396055
🤖 *AI text below* 🤖 *(reviewed by Nick Shanin)* Hi @isaric, I have a tested follow-up that adds missing coverage for the per-shard replication-factor API: [diff](https://github.com/nick-boss-tech/solr/compare/7f9acb9df610338c20537e1efd6ed75a6c64a13b...b244ae94d6c84eec06bf1de5754e566b0fdb88d1). The current tests exercise the minimum factor, but nothing pins the full shard-to-factor map the API returns, so a regression that returned the same factor for every shard would pass unnoticed. It adds the missing tests to `CloudHttp2SolrClientTest`, test-only, with no production changes: `testShardReplicationFactors` feeds distinct per-shard factors (2 and 3) through real shard leaders and checks the full shard-to-factor map the public API returns, and `testShardReplicationFactorsWithoutRouteFactors` covers the omitted-metadata case. The existing `checkSingleServer` helper now asserts the per-shard map for its real updates too. <details> <summary>Verification on Linux at `b244ae94d6` (JDK 21.0.12.1, Gradle 9.7.0, seed `18603C0DE`)</summary> - `CloudHttp2SolrClientTest` passes in full (22 tests, including the two new ones) and `NestableJsonFacetTest` passes; tidy is clean and root `check -x test` passes. - The new assertions bite: with the production code made to drop the per-shard factors from the response, the first test fails on the missing factors; with the factors corrupted to 0, it fails on the map. </details> The commits are a fast-forward of your current PR head: ```sh git switch SOLR-18603-seal-namedlist git fetch https://github.com/nick-boss-tech/solr.git solr-18603-submit git merge --ff-only b244ae94d6c84eec06bf1de5754e566b0fdb88d1 ``` You're welcome to take the commits or adapt the changes. If you would rather I open it as a separate PR after this one merges, I can do that instead. Separately, the Crave failure in `TestCloudConsistency` appears unrelated to this change: I could not reproduce it on your head or on main with the recorded seed, so it should clear on a rerun. -- 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]
