Remove relative date in IndicesRequestCacheIT

[andrross]

The dates in the test are starting to fall out of the now-10y window. It should be fine to use a fixed date for the 'from' since the 'to' uses now.

Check List

• Functionality includes testing.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 29a5e3a)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Test Semantics The replacement of now-10y with 2016-01-01 changes the nature of the test. The original intent was to use a relative date expression containing now to verify caching behavior. With a fixed date 2016-01-01, the range addRange("2016-01-01", "now") still contains now at the end, so the non-caching behavior for r5 should still hold. However, it's worth verifying that the fixed start date 2016-01-01 still correctly covers the test data (indexed around 2016-03-20 to 2016-03-26) and that the expected hit counts remain valid. .addAggregation(dateRange("foo").field("s").addRange("2016-01-01", "now")) .get(); OpenSearchAssertions.assertAllSuccessful(r5); assertThat(r5.getHits().getTotalHits().value(), equalTo(7L)); assertCacheState(client, index, 0, 0); // If size > 1 and cache flag is set on the request we should cache final SearchResponse r6 = client.prepareSearch(index) .setSearchType(SearchType.QUERY_THEN_FETCH) .setSize(1) .setRequestCache(true) .setQuery(QueryBuilders.rangeQuery("s").gte("2016-03-21").lte("2016-03-27")) .get(); OpenSearchAssertions.assertAllSuccessful(r6); assertThat(r6.getHits().getTotalHits().value(), equalTo(7L)); assertCacheState(client, index, 0, 2); // If the request has a filter aggregation containing now we should cache since it gets rewritten final SearchResponse r4 = client.prepareSearch(index) .setSearchType(SearchType.QUERY_THEN_FETCH) .setSize(0) .setRequestCache(true) .setQuery(QueryBuilders.rangeQuery("s").gte("2016-03-20").lte("2016-03-26")) .addAggregation(filter("foo", QueryBuilders.rangeQuery("s").from("2016-01-01").to("now")))

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 29a5e3a
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Replace remaining relative date with absolute date The test comment says "non-filter aggregation containing now we should not cache", but the range still contains "now" as the upper bound. If the intent is to remove relative dates to avoid flakiness, both bounds should be absolute dates. Using "now" still introduces a relative date that could cause the test to behave differently over time. server/src/internalClusterTest/java/org/opensearch/indices/IndicesRequestCacheIT.java [555] -.addAggregation(dateRange("foo").field("s").addRange("2016-01-01", "now")) +.addAggregation(dateRange("foo").field("s").addRange("2016-01-01", "2016-03-27")) Suggestion importance[1-10]: 5 __ Why: The suggestion raises a valid concern about using "now" in the test, but the test comment explicitly states "non-filter aggregation containing now we should not cache" - meaning the presence of "now" is intentional to test that behavior. The PR changed from "now-10y" to "2016-01-01" for the lower bound while keeping "now" to maintain the test's purpose of verifying that "now" prevents caching.	Low

---

Previous suggestions

Suggestions up to commit 14d37cd

Category	Suggestion	Impact
Possible issue	Replace remaining relative date with absolute date The test comment says "non-filter aggregation containing now we should not cache", but the range still ends with "now" which is a relative date. If the intent is to remove relative dates entirely, both bounds should be absolute dates. If "now" is intentionally kept to trigger the no-cache behavior, the comment and the symmetry with the other fix should be reconsidered. server/src/internalClusterTest/java/org/opensearch/indices/IndicesRequestCacheIT.java [555] -.addAggregation(dateRange("foo").field("s").addRange("2016-01-01", "now")) +.addAggregation(dateRange("foo").field("s").addRange("2016-01-01", "2016-03-26")) Suggestion importance[1-10]: 3 __ Why: The suggestion raises a valid point about consistency, but the test comment explicitly states "non-filter aggregation containing now we should not cache" - keeping "now" in the range is intentional to trigger the no-cache behavior. Replacing it with an absolute date would change the test's purpose and potentially break the test logic.	Low

[github-actions]

❌ Gradle check result for 14d37cd: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 29a5e3a

[github-actions]

❌ Gradle check result for 29a5e3a: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[kkewwei]

LGTM.

After analysis, the root cause is as follows:

• Previously, the time range now-10y ~ now had an INTERSECTS relation with the data set (originally WITHIN). After the rewrite, from-to becomes null, so the second time parsing is skipped and the query should be cacheable = true.

• However, since the relation between now-10y ~ now and the data set is still INTERSECTS, parseSource will rewrite the dynamic time now again, which results in cacheable = false.

[github-actions]

✅ Gradle check result for 29a5e3a: SUCCESS

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.21%. Comparing base (9f5d0e1) to head (29a5e3a).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #20920      +/-   ##
============================================
- Coverage     73.30%   73.21%   -0.10%     
+ Complexity    72484    72477       -7     
============================================
  Files          5819     5819              
  Lines        331155   331237      +82     
  Branches      47840    47860      +20     
============================================
- Hits         242769   242531     -238     
- Misses        68876    69227     +351     
+ Partials      19510    19479      -31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[cwperks]

How many other ticking time bombs do we have in the core or any other repo?

Speaking for security, I know we have some demo certs that expire in the future but for the most part the repo will generate certs on the fly for testing.