Skip to content

Commit 4a1cbef

Browse files
committed
test: Refactor unified query profiling tests with shared test base
Extract common test schema and utilities into UnifiedQueryTestBase to reduce duplication across unified query test classes. Profiling tests now extend this base and override buildContext() to enable profiling. Change all timing assertions from > 0 to >= 0 to prevent flakiness. Millisecond-resolution timers can return 0 for fast operations when System.currentTimeMillis() samples the same tick before and after. The >= 0 assertion still validates the metric was recorded (phase key exists in the profile map) without depending on wall-clock granularity. Signed-off-by: Chen Dai <daichen@amazon.com>
1 parent a9cdf7b commit 4a1cbef

2 files changed

Lines changed: 52 additions & 91 deletions

File tree

api/src/test/java/org/opensearch/sql/api/UnifiedQueryProfilingTest.java

Lines changed: 41 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
import static org.junit.Assert.assertEquals;
99
import static org.junit.Assert.assertFalse;
10-
import static org.junit.Assert.assertNotNull;
1110
import static org.junit.Assert.assertNull;
1211
import static org.junit.Assert.assertThrows;
1312
import static org.junit.Assert.assertTrue;
@@ -21,143 +20,99 @@
2120
import org.junit.Test;
2221
import org.opensearch.sql.api.compiler.UnifiedQueryCompiler;
2322
import org.opensearch.sql.api.transpiler.UnifiedQueryTranspiler;
24-
import org.opensearch.sql.executor.QueryType;
2523
import org.opensearch.sql.monitor.profile.QueryProfile;
2624
import org.opensearch.sql.monitor.profile.QueryProfiling;
2725

2826
/** Tests for profiling across unified query components and the measure() API. */
2927
public class UnifiedQueryProfilingTest extends UnifiedQueryTestBase {
3028

31-
@Test
32-
public void testProfilingEnabled() throws Exception {
33-
try (UnifiedQueryContext ctx = buildContext(true)) {
34-
assertTrue(QueryProfiling.current().isEnabled());
35-
}
29+
@Override
30+
protected UnifiedQueryContext.Builder buildContext() {
31+
return super.buildContext().profiling(true);
3632
}
3733

3834
@Test
39-
public void testProfilingDisabledByDefault() throws Exception {
40-
try (UnifiedQueryContext ctx =
41-
UnifiedQueryContext.builder()
42-
.language(QueryType.PPL)
43-
.catalog("opensearch", testSchema)
44-
.defaultNamespace("opensearch")
45-
.build()) {
46-
assertFalse(QueryProfiling.current().isEnabled());
47-
}
35+
public void testProfilingEnabled() {
36+
assertTrue(QueryProfiling.current().isEnabled());
4837
}
4938

5039
@Test
51-
public void testProfilingExplicitlyDisabled() throws Exception {
52-
try (UnifiedQueryContext ctx = buildContext(false)) {
40+
public void testProfilingDisabledByDefault() throws Exception {
41+
try (UnifiedQueryContext ctx = super.buildContext().build()) {
5342
assertFalse(QueryProfiling.current().isEnabled());
5443
}
5544
}
5645

5746
@Test
5847
public void testProfilingClearedAfterClose() throws Exception {
59-
UnifiedQueryContext ctx = buildContext(true);
6048
assertTrue(QueryProfiling.current().isEnabled());
61-
ctx.close();
49+
context.close();
6250
assertFalse(QueryProfiling.current().isEnabled());
6351
}
6452

6553
@Test
6654
public void testGetProfileReturnsNullWhenDisabled() throws Exception {
67-
try (UnifiedQueryContext ctx = buildContext(false)) {
55+
try (UnifiedQueryContext ctx = super.buildContext().build()) {
6856
assertNull(ctx.getProfile());
6957
}
7058
}
7159

7260
@Test
73-
public void testPlannerAutoProfilesAnalyzePhase() throws Exception {
74-
try (UnifiedQueryContext ctx = buildContext(true)) {
75-
new UnifiedQueryPlanner(ctx).plan("source = opensearch.employees");
76-
QueryProfile profile = ctx.getProfile();
77-
assertNotNull(profile);
78-
assertTrue(profile.getPhases().get("analyze").getTimeMillis() > 0);
79-
}
61+
public void testPlannerAutoProfilesAnalyzePhase() {
62+
planner.plan("source = catalog.employees");
63+
assertTrue(context.getProfile().getPhases().get("analyze").getTimeMillis() >= 0);
8064
}
8165

8266
@Test
83-
public void testCompilerAutoProfilesOptimizePhase() throws Exception {
84-
try (UnifiedQueryContext ctx = buildContext(true)) {
85-
RelNode plan = new UnifiedQueryPlanner(ctx).plan("source = opensearch.employees");
86-
PreparedStatement stmt = new UnifiedQueryCompiler(ctx).compile(plan);
87-
assertNotNull(stmt);
88-
QueryProfile profile = ctx.getProfile();
89-
assertNotNull(profile);
90-
assertTrue(profile.getPhases().get("optimize").getTimeMillis() > 0);
91-
}
67+
public void testCompilerAutoProfilesOptimizePhase() {
68+
RelNode plan = planner.plan("source = catalog.employees");
69+
new UnifiedQueryCompiler(context).compile(plan);
70+
assertTrue(context.getProfile().getPhases().get("optimize").getTimeMillis() >= 0);
9271
}
9372

9473
@Test
95-
public void testTranspilerAutoProfilesTranspilePhase() throws Exception {
96-
try (UnifiedQueryContext ctx = buildContext(true)) {
97-
RelNode plan = new UnifiedQueryPlanner(ctx).plan("source = opensearch.employees");
98-
UnifiedQueryTranspiler transpiler = new UnifiedQueryTranspiler(ctx, SparkSqlDialect.DEFAULT);
99-
assertNotNull(transpiler.toSql(plan));
100-
QueryProfile profile = ctx.getProfile();
101-
assertNotNull(profile);
102-
assertTrue(profile.getPhases().get("transpile").getTimeMillis() > 0);
103-
}
74+
public void testTranspilerAutoProfilesTranspilePhase() {
75+
RelNode plan = planner.plan("source = catalog.employees");
76+
new UnifiedQueryTranspiler(context, SparkSqlDialect.DEFAULT).toSql(plan);
77+
assertTrue(context.getProfile().getPhases().get("transpile").getTimeMillis() >= 0);
10478
}
10579

10680
@Test
10781
public void testMeasureRecordsMetric() throws Exception {
108-
try (UnifiedQueryContext ctx = buildContext(true)) {
109-
assertEquals("done", ctx.measure(EXECUTE, () -> "done"));
110-
QueryProfile profile = ctx.getProfile();
111-
assertNotNull(profile);
112-
assertTrue(profile.getPhases().get("execute").getTimeMillis() >= 0);
113-
}
82+
assertEquals("done", context.measure(EXECUTE, () -> "done"));
83+
assertTrue(context.getProfile().getPhases().get("execute").getTimeMillis() >= 0);
11484
}
11585

11686
@Test
11787
public void testMeasureExecutesWhenProfilingDisabled() throws Exception {
118-
try (UnifiedQueryContext ctx = buildContext(false)) {
88+
try (UnifiedQueryContext ctx = super.buildContext().build()) {
11989
assertEquals("done", ctx.measure(EXECUTE, () -> "done"));
12090
assertNull(ctx.getProfile());
12191
}
12292
}
12393

12494
@Test
125-
public void testMeasurePropagatesException() throws Exception {
126-
try (UnifiedQueryContext ctx = buildContext(true)) {
127-
assertThrows(
128-
IOException.class,
129-
() ->
130-
ctx.measure(
131-
EXECUTE,
132-
() -> {
133-
throw new IOException("test error");
134-
}));
135-
}
95+
public void testMeasurePropagatesException() {
96+
assertThrows(
97+
IOException.class,
98+
() ->
99+
context.measure(
100+
EXECUTE,
101+
() -> {
102+
throw new IOException("test error");
103+
}));
136104
}
137105

138106
@Test
139107
public void testFullPipelineProfiling() throws Exception {
140-
try (UnifiedQueryContext ctx = buildContext(true)) {
141-
RelNode plan = new UnifiedQueryPlanner(ctx).plan("source = opensearch.employees");
142-
PreparedStatement stmt = new UnifiedQueryCompiler(ctx).compile(plan);
143-
ResultSet rs = ctx.measure(EXECUTE, stmt::executeQuery);
144-
assertNotNull(rs);
145-
146-
QueryProfile profile = ctx.getProfile();
147-
assertNotNull(profile);
148-
assertTrue(profile.getSummary().getTotalTimeMillis() > 0);
149-
assertTrue(profile.getPhases().get("analyze").getTimeMillis() > 0);
150-
assertTrue(profile.getPhases().get("optimize").getTimeMillis() > 0);
151-
assertTrue(profile.getPhases().get("execute").getTimeMillis() >= 0);
152-
}
153-
}
154-
155-
private UnifiedQueryContext buildContext(boolean profiling) {
156-
return UnifiedQueryContext.builder()
157-
.language(QueryType.PPL)
158-
.catalog("opensearch", testSchema)
159-
.defaultNamespace("opensearch")
160-
.profiling(profiling)
161-
.build();
108+
RelNode plan = planner.plan("source = catalog.employees");
109+
PreparedStatement stmt = new UnifiedQueryCompiler(context).compile(plan);
110+
ResultSet rs = context.measure(EXECUTE, stmt::executeQuery);
111+
112+
QueryProfile profile = context.getProfile();
113+
assertTrue(profile.getSummary().getTotalTimeMillis() >= 0);
114+
assertTrue(profile.getPhases().get("analyze").getTimeMillis() >= 0);
115+
assertTrue(profile.getPhases().get("optimize").getTimeMillis() >= 0);
116+
assertTrue(profile.getPhases().get("execute").getTimeMillis() >= 0);
162117
}
163118
}

api/src/testFixtures/java/org/opensearch/sql/api/UnifiedQueryTestBase.java

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,20 @@ protected Map<String, Table> getTableMap() {
5555
}
5656
};
5757

58-
context =
59-
UnifiedQueryContext.builder()
60-
.language(QueryType.PPL)
61-
.catalog(DEFAULT_CATALOG, testSchema)
62-
.build();
58+
context = buildContext().build();
6359
planner = new UnifiedQueryPlanner(context);
6460
}
6561

62+
/**
63+
* Creates a pre-configured context builder with test schema. Subclasses can override to customize
64+
* context configuration (e.g., enable profiling).
65+
*/
66+
protected UnifiedQueryContext.Builder buildContext() {
67+
return UnifiedQueryContext.builder()
68+
.language(QueryType.PPL)
69+
.catalog(DEFAULT_CATALOG, testSchema);
70+
}
71+
6672
@After
6773
public void tearDown() throws Exception {
6874
if (context != null) {

0 commit comments

Comments
 (0)