nick-boss-tech opened a new pull request, #5089: URL: https://github.com/apache/solr/pull/5089
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)* https://issues.apache.org/jira/browse/SOLR-13943 ## What happens today **`testDateMathInStart` does not run in normal CI. When it runs, it waits on the test's own ZooKeeper watcher.** The method sits in `TimeRoutedAliasUpdateProcessorTest`. That class has a class-level `@AwaitsFix` for SOLR-13059 ([annotation](https://github.com/nick-boss-tech/solr/blob/cc155cf68e1d8e79f1bcecd2e4c25ada864d8f53/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java#L69-L70)), so its methods are skipped in normal mode. The method also had its own `@AwaitsFix` for SOLR-13943. After the add, the test waits on a latch that counts down when its own watcher on `/aliases.json` fires. Then it reads the start value from the cluster state provider. ## What this change does **The test moves to its own class with no skip annotation. Its second wait now polls the provider.** - The method moves to `TimeRoutedAliasDateMathInStartTest`, a new class. It has the same base class and no `@AwaitsFix` ([class](https://github.com/nick-boss-tech/solr/blob/cc155cf68e1d8e79f1bcecd2e4c25ada864d8f53/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasDateMathInStartTest.java#L51)). The test is at [L84-L160](https://github.com/nick-boss-tech/solr/blob/cc155cf68e1d8e79f1bcecd2e4c25ada864d8f53/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasDateMathInStartTest.java#L84-L160). - The wait after the add now polls the cluster state provider every 100 ms, for up to 30 seconds. On an HTTP-based provider, it first asks the provider to refresh the alias. The test passes once the start value parses as an instant. It fails at the timeout ([poll loop](https://github.com/nick-boss-tech/solr/blob/cc155cf68e1d8e79f1bcecd2e4c25ada864d8f53/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasDateMathInStartTest.java#L135-L159)). The old wait had no time limit. - The first wait, before the add, still uses the test's own watcher ([L109](https://github.com/nick-boss-tech/solr/blob/cc155cf68e1d8e79f1bcecd2e4c25ada864d8f53/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasDateMathInStartTest.java#L109)). This change leaves it as it was. - The new comment gives the reason. The watcher can fire before the provider has refreshed its aliases ([comment](https://github.com/nick-boss-tech/solr/blob/cc155cf68e1d8e79f1bcecd2e4c25ada864d8f53/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasDateMathInStartTest.java#L132-L134)). - The change touches tests only. No production code changes. ## Proof **The moved test passes in normal mode, and the nearby suites pass too.** - `TimeRoutedAliasDateMathInStartTest` [`testDateMathInStart`](https://github.com/nick-boss-tech/solr/blob/cc155cf68e1d8e79f1bcecd2e4c25ada864d8f53/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasDateMathInStartTest.java#L84-L160): 1 of 1 in normal mode. - `CategoryRoutedAliasUpdateProcessorTest` 6 of 6, `DimensionalRoutedAliasUpdateProcessorTest` 2 of 2, and `CreateAliasAPITest` 13 of 13, all in normal mode. - Tidy exits 0 with a clean tree. The Error Prone compile exits 0. `:solr:core:check -x test` exits 0. These counts come from fresh JUnit XML at this head. With the `awaitsfix` group enabled, `TimeRoutedAliasUpdateProcessorTest` runs six tests. Five pass. [`testPreemptiveCreation`](https://github.com/nick-boss-tech/solr/blob/cc155cf68e1d8e79f1bcecd2e4c25ada864d8f53/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java#L301-L302) fails. This failure is pre-existing, and this change does not cause it. The change does not touch that test. Before this branch was stacked on SOLR-13696, the same test failed in 3 of 5 runs. The failure recorded was an expected 3 and an actual 4 in `concurrentUpdates` ([assertion](https://github.com/nick-boss-tech/solr/blob/cc155cf68e1d8e79f1bcecd2e4c25ada864d8f53/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java#L705)). The method's [comment](https://github.com/nick-boss-tech/solr/blob/cc155cf68e1d8e79f1bcecd2e4c25ada864d8f53/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java#L678-L683) s ays it intentionally relies on the timing of a race condition. This change does not fix that test. ## Limits **The original class stays skipped under SOLR-13059. This change does not fix SOLR-13059.** - The six tests left in `TimeRoutedAliasUpdateProcessorTest` keep the class-level `@AwaitsFix`, so they stay skipped in normal mode. `testPreemptiveCreation` is one of them. - No run of the moved test against the base code is on record. This PR does not yet show that the test fails without the change. - This branch is stacked on the SOLR-13696 branch at `1d0b8a0a73cd`. That base stays, because this branch does not contain the changelog commit. The SOLR-13696 branch has since gained one commit, `da4fa6df117`. It adds the changelog fragment and changes no file this branch touches. This PR opens after, or alongside, the SOLR-13696 PR. If SOLR-13696 lands first, this branch rebases onto main, and no content change is expected. Changelog: this branch adds no changelog fragment. ### AI assistance AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution. -- 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]
