Skip to content

Commit 256f812

Browse files
YarivHashaiCometYarivHashaiComet
committed
fixing tests
1 parent c4d0ef7 commit 256f812

File tree

2 files changed

+314
-1
lines changed

2 files changed

+314
-1
lines changed

‎apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/events/OnlineScoringEngineTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,7 @@ private AutomationRuleEvaluatorLlmAsJudge createRule(UUID projectId, LlmAsJudgeC
395395
.createdBy(USER_NAME)
396396
.code(evaluatorCode)
397397
.samplingRate(1.0f)
398+
.enabled(true)
398399
.build();
399400
}
400401

‎apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AutomationRuleEvaluatorsResourceTest.java

Lines changed: 313 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -791,7 +791,8 @@ Stream<Arguments> update() {
791791
factory.manufacturePojo(AutomationRuleEvaluatorUpdateLlmAsJudge.class)),
792792
arguments(
793793
factory.manufacturePojo(AutomationRuleEvaluatorUserDefinedMetricPython.class),
794-
factory.manufacturePojo(AutomationRuleEvaluatorUpdateUserDefinedMetricPython.class)),
794+
factory.manufacturePojo(
795+
AutomationRuleEvaluatorUpdateUserDefinedMetricPython.class)),
795796
arguments(
796797
factory.manufacturePojo(AutomationRuleEvaluatorTraceThreadLlmAsJudge.class),
797798
factory.manufacturePojo(AutomationRuleEvaluatorUpdateTraceThreadLlmAsJudge.class)),
@@ -822,6 +823,7 @@ void update(
822823
.lastUpdatedBy(USER)
823824
.name(updatedEvaluator.getName())
824825
.samplingRate(updatedEvaluator.getSamplingRate())
826+
.enabled(updatedEvaluator.isEnabled())
825827
.code(updatedEvaluator.getCode())
826828
.build();
827829
try (var actualResponse = evaluatorsResourceClient.getEvaluator(
@@ -834,6 +836,66 @@ void update(
834836
assertIgnoredFields(actualAutomationRuleEvaluator, expectedAutomationRuleEvaluator);
835837
}
836838
}
839+
840+
@Test
841+
void updateEnabledToDisabled() {
842+
// Create an enabled rule
843+
var automationRuleEvaluator = factory.manufacturePojo(AutomationRuleEvaluatorLlmAsJudge.class)
844+
.toBuilder()
845+
.enabled(true)
846+
.build();
847+
var id = evaluatorsResourceClient.createEvaluator(automationRuleEvaluator, WORKSPACE_NAME, API_KEY);
848+
849+
// Update to disable it
850+
var updatedEvaluator = factory.manufacturePojo(AutomationRuleEvaluatorUpdateLlmAsJudge.class)
851+
.toBuilder()
852+
.projectId(automationRuleEvaluator.getProjectId())
853+
.enabled(false)
854+
.build();
855+
856+
try (var actualResponse = evaluatorsResourceClient.updateEvaluator(
857+
id, WORKSPACE_NAME, updatedEvaluator, API_KEY, HttpStatus.SC_NO_CONTENT)) {
858+
assertThat(actualResponse.hasEntity()).isFalse();
859+
}
860+
861+
// Verify the rule is now disabled
862+
try (var actualResponse = evaluatorsResourceClient.getEvaluator(
863+
id, null, WORKSPACE_NAME, API_KEY, HttpStatus.SC_OK)) {
864+
var actualAutomationRuleEvaluator = actualResponse.readEntity(AutomationRuleEvaluator.class);
865+
assertThat(actualAutomationRuleEvaluator.isEnabled()).isFalse();
866+
assertThat(actualAutomationRuleEvaluator.getName()).isEqualTo(updatedEvaluator.getName());
867+
}
868+
}
869+
870+
@Test
871+
void updateDisabledToEnabled() {
872+
// Create a disabled rule
873+
var automationRuleEvaluator = factory.manufacturePojo(AutomationRuleEvaluatorLlmAsJudge.class)
874+
.toBuilder()
875+
.enabled(false)
876+
.build();
877+
var id = evaluatorsResourceClient.createEvaluator(automationRuleEvaluator, WORKSPACE_NAME, API_KEY);
878+
879+
// Update to enable it
880+
var updatedEvaluator = factory.manufacturePojo(AutomationRuleEvaluatorUpdateLlmAsJudge.class)
881+
.toBuilder()
882+
.projectId(automationRuleEvaluator.getProjectId())
883+
.enabled(true)
884+
.build();
885+
886+
try (var actualResponse = evaluatorsResourceClient.updateEvaluator(
887+
id, WORKSPACE_NAME, updatedEvaluator, API_KEY, HttpStatus.SC_NO_CONTENT)) {
888+
assertThat(actualResponse.hasEntity()).isFalse();
889+
}
890+
891+
// Verify the rule is now enabled
892+
try (var actualResponse = evaluatorsResourceClient.getEvaluator(
893+
id, null, WORKSPACE_NAME, API_KEY, HttpStatus.SC_OK)) {
894+
var actualAutomationRuleEvaluator = actualResponse.readEntity(AutomationRuleEvaluator.class);
895+
assertThat(actualAutomationRuleEvaluator.isEnabled()).isTrue();
896+
assertThat(actualAutomationRuleEvaluator.getName()).isEqualTo(updatedEvaluator.getName());
897+
}
898+
}
837899
}
838900

839901
@Nested
@@ -1170,6 +1232,221 @@ void getLogsTraceThreadSkipped() {
11701232

11711233
}
11721234

1235+
@Nested
1236+
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
1237+
class DisabledRulesTest {
1238+
1239+
@Test
1240+
void createEvaluatorWithDefaultEnabledTrue() {
1241+
// Create a rule without explicitly setting enabled - should default to true
1242+
var automationRuleEvaluator = factory.manufacturePojo(AutomationRuleEvaluatorLlmAsJudge.class);
1243+
var id = evaluatorsResourceClient.createEvaluator(automationRuleEvaluator, WORKSPACE_NAME, API_KEY);
1244+
1245+
try (var actualResponse = evaluatorsResourceClient.getEvaluator(
1246+
id, null, WORKSPACE_NAME, API_KEY, HttpStatus.SC_OK)) {
1247+
var actualAutomationRuleEvaluator = actualResponse.readEntity(AutomationRuleEvaluator.class);
1248+
// Should default to enabled if not explicitly set
1249+
assertThat(actualAutomationRuleEvaluator.isEnabled()).isTrue();
1250+
}
1251+
}
1252+
1253+
@Test
1254+
void createDisabledEvaluator() {
1255+
// Create a disabled rule explicitly
1256+
var automationRuleEvaluator = factory.manufacturePojo(AutomationRuleEvaluatorLlmAsJudge.class)
1257+
.toBuilder()
1258+
.enabled(false)
1259+
.build();
1260+
var id = evaluatorsResourceClient.createEvaluator(automationRuleEvaluator, WORKSPACE_NAME, API_KEY);
1261+
1262+
try (var actualResponse = evaluatorsResourceClient.getEvaluator(
1263+
id, null, WORKSPACE_NAME, API_KEY, HttpStatus.SC_OK)) {
1264+
var actualAutomationRuleEvaluator = actualResponse.readEntity(AutomationRuleEvaluator.class);
1265+
assertThat(actualAutomationRuleEvaluator.isEnabled()).isFalse();
1266+
}
1267+
}
1268+
1269+
@Test
1270+
void getLogsTraceSkippedDueToDisabledRule() {
1271+
// Test that disabled rules generate appropriate log messages (different from sampling rate messages)
1272+
var projectName = "project-" + RandomStringUtils.randomAlphanumeric(36);
1273+
var projectId = projectResourceClient.createProject(projectName, API_KEY, WORKSPACE_NAME);
1274+
1275+
// Create a disabled Python rule with high sampling rate (to distinguish from sampling rate issues)
1276+
var evaluatorPython = factory.manufacturePojo(AutomationRuleEvaluatorUserDefinedMetricPython.class)
1277+
.toBuilder()
1278+
.samplingRate(1.0f) // High sampling rate, but rule is disabled
1279+
.enabled(false)
1280+
.projectId(projectId)
1281+
.build();
1282+
var idPython = evaluatorsResourceClient.createEvaluator(evaluatorPython, WORKSPACE_NAME, API_KEY);
1283+
1284+
// Create a disabled LLM rule with high sampling rate
1285+
var evaluatorLlm = factory.manufacturePojo(AutomationRuleEvaluatorLlmAsJudge.class)
1286+
.toBuilder()
1287+
.samplingRate(1.0f) // High sampling rate, but rule is disabled
1288+
.enabled(false)
1289+
.projectId(projectId)
1290+
.build();
1291+
var idLlm = evaluatorsResourceClient.createEvaluator(evaluatorLlm, WORKSPACE_NAME, API_KEY);
1292+
1293+
// Send a trace that should be skipped due to disabled rules (not sampling rate)
1294+
var trace = factory.manufacturePojo(Trace.class).toBuilder()
1295+
.projectName(projectName)
1296+
.threadId(null)
1297+
.build();
1298+
traceResourceClient.createTrace(trace, API_KEY, WORKSPACE_NAME);
1299+
1300+
Awaitility.await().untilAsserted(() -> {
1301+
var logPagePython = evaluatorsResourceClient.getLogs(idPython, WORKSPACE_NAME, API_KEY);
1302+
assertDisabledRuleLogResponse(logPagePython, idPython, evaluatorPython, trace);
1303+
var logPageLlm = evaluatorsResourceClient.getLogs(idLlm, WORKSPACE_NAME, API_KEY);
1304+
assertDisabledRuleLogResponse(logPageLlm, idLlm, evaluatorLlm, trace);
1305+
});
1306+
}
1307+
1308+
@Test
1309+
void getLogsTraceThreadSkippedDueToDisabledRule() {
1310+
// Test trace thread handling for disabled rules
1311+
var projectName = "project-" + RandomStringUtils.randomAlphanumeric(36);
1312+
var projectId = projectResourceClient.createProject(projectName, API_KEY, WORKSPACE_NAME);
1313+
1314+
// Create disabled trace thread rules with high sampling rates
1315+
var evaluatorPython = Optional.ofNullable(factory
1316+
.manufacturePojo(AutomationRuleEvaluatorTraceThreadUserDefinedMetricPython.class)
1317+
.toBuilder()
1318+
.samplingRate(1.0f) // High sampling rate, but rule is disabled
1319+
.enabled(false)
1320+
.projectId(projectId)
1321+
.build())
1322+
.map(evaluator -> evaluator.toBuilder()
1323+
.id(evaluatorsResourceClient.createEvaluator(evaluator, WORKSPACE_NAME, API_KEY))
1324+
.build())
1325+
.get();
1326+
1327+
var evaluatorLlm = Optional.ofNullable(
1328+
factory.manufacturePojo(AutomationRuleEvaluatorTraceThreadLlmAsJudge.class)
1329+
.toBuilder()
1330+
.samplingRate(1.0f) // High sampling rate, but rule is disabled
1331+
.enabled(false)
1332+
.projectId(projectId)
1333+
.build())
1334+
.map(evaluator -> evaluator.toBuilder()
1335+
.id(evaluatorsResourceClient.createEvaluator(evaluator, WORKSPACE_NAME, API_KEY))
1336+
.build())
1337+
.get();
1338+
1339+
// Send a trace that should create a thread but be skipped due to disabled rules
1340+
var trace = factory.manufacturePojo(Trace.class).toBuilder()
1341+
.projectName(projectName)
1342+
.build();
1343+
1344+
Instant createdAt = trace.createdAt();
1345+
traceResourceClient.createTrace(trace, API_KEY, WORKSPACE_NAME);
1346+
1347+
Awaitility.await().untilAsserted(() -> {
1348+
TraceThread traceThread = traceResourceClient.getTraceThread(trace.threadId(), projectId, API_KEY,
1349+
WORKSPACE_NAME);
1350+
1351+
var logPagePython = evaluatorsResourceClient.getLogs(evaluatorPython.getId(), WORKSPACE_NAME, API_KEY);
1352+
var logPageLlm = evaluatorsResourceClient.getLogs(evaluatorLlm.getId(), WORKSPACE_NAME, API_KEY);
1353+
1354+
String threadModelId = Optional.ofNullable(traceThread.threadModelId())
1355+
.map(Object::toString)
1356+
.orElseThrow();
1357+
1358+
Map<String, String> markers = Map.of("thread_model_id", threadModelId);
1359+
String disabledMessage = "The threadModelId '%s' was skipped for rule: '%s' as the rule is disabled";
1360+
1361+
assertTraceThreadDisabledLogResponse(logPagePython, threadModelId, evaluatorPython, markers, createdAt,
1362+
disabledMessage);
1363+
assertTraceThreadDisabledLogResponse(logPageLlm, threadModelId, evaluatorLlm, markers, createdAt,
1364+
disabledMessage);
1365+
});
1366+
}
1367+
1368+
@Test
1369+
void mixedEnabledAndDisabledRules() {
1370+
// Test scenario with both enabled and disabled rules in same project
1371+
var projectName = "project-" + RandomStringUtils.randomAlphanumeric(36);
1372+
var projectId = projectResourceClient.createProject(projectName, API_KEY, WORKSPACE_NAME);
1373+
1374+
// Create one enabled rule
1375+
var enabledRule = factory.manufacturePojo(AutomationRuleEvaluatorLlmAsJudge.class)
1376+
.toBuilder()
1377+
.samplingRate(0f) // Low sampling rate to avoid actual processing
1378+
.enabled(true)
1379+
.projectId(projectId)
1380+
.build();
1381+
var enabledId = evaluatorsResourceClient.createEvaluator(enabledRule, WORKSPACE_NAME, API_KEY);
1382+
1383+
// Create one disabled rule
1384+
var disabledRule = factory.manufacturePojo(AutomationRuleEvaluatorUserDefinedMetricPython.class)
1385+
.toBuilder()
1386+
.samplingRate(1.0f) // High sampling rate, but rule is disabled
1387+
.enabled(false)
1388+
.projectId(projectId)
1389+
.build();
1390+
var disabledId = evaluatorsResourceClient.createEvaluator(disabledRule, WORKSPACE_NAME, API_KEY);
1391+
1392+
// Send a trace
1393+
var trace = factory.manufacturePojo(Trace.class).toBuilder()
1394+
.projectName(projectName)
1395+
.threadId(null)
1396+
.build();
1397+
traceResourceClient.createTrace(trace, API_KEY, WORKSPACE_NAME);
1398+
1399+
Awaitility.await().untilAsserted(() -> {
1400+
// Enabled rule should generate sampling rate message (skipped due to 0% rate)
1401+
var enabledLogPage = evaluatorsResourceClient.getLogs(enabledId, WORKSPACE_NAME, API_KEY);
1402+
assertLogResponse(enabledLogPage, enabledId, enabledRule, trace);
1403+
1404+
// Disabled rule should generate disabled message
1405+
var disabledLogPage = evaluatorsResourceClient.getLogs(disabledId, WORKSPACE_NAME, API_KEY);
1406+
assertDisabledRuleLogResponse(disabledLogPage, disabledId, disabledRule, trace);
1407+
});
1408+
}
1409+
1410+
@Test
1411+
void disabledRuleDoesNotConsumeResources() {
1412+
// Verify that disabled rules are skipped early and don't go through sampling logic
1413+
var projectName = "project-" + RandomStringUtils.randomAlphanumeric(36);
1414+
var projectId = projectResourceClient.createProject(projectName, API_KEY, WORKSPACE_NAME);
1415+
1416+
// Create disabled rule with 100% sampling rate
1417+
var disabledRule = factory.manufacturePojo(AutomationRuleEvaluatorLlmAsJudge.class)
1418+
.toBuilder()
1419+
.samplingRate(1.0f) // 100% sampling rate
1420+
.enabled(false) // But disabled
1421+
.projectId(projectId)
1422+
.build();
1423+
var disabledId = evaluatorsResourceClient.createEvaluator(disabledRule, WORKSPACE_NAME, API_KEY);
1424+
1425+
// Send multiple traces
1426+
for (int i = 0; i < 5; i++) {
1427+
var trace = factory.manufacturePojo(Trace.class).toBuilder()
1428+
.projectName(projectName)
1429+
.threadId(null)
1430+
.build();
1431+
traceResourceClient.createTrace(trace, API_KEY, WORKSPACE_NAME);
1432+
}
1433+
1434+
// All traces should be skipped with "disabled" message, none with sampling rate message
1435+
Awaitility.await().untilAsserted(() -> {
1436+
var logPage = evaluatorsResourceClient.getLogs(disabledId, WORKSPACE_NAME, API_KEY);
1437+
assertLogPage(logPage, 5); // Should have 5 log entries
1438+
1439+
// All log messages should be about rule being disabled, not sampling rate
1440+
assertThat(logPage.content())
1441+
.allSatisfy(log -> {
1442+
assertThat(log.level()).isEqualTo(LogLevel.INFO);
1443+
assertThat(log.message()).contains("as the rule is disabled");
1444+
assertThat(log.message()).doesNotContain("per the sampling rate");
1445+
});
1446+
});
1447+
}
1448+
}
1449+
11731450
private void assertTraceThreadLogResponse(LogPage logPage, String threadModelId, AutomationRuleEvaluator<?> rule,
11741451
Map<String, String> markers,
11751452
Instant createdAt, String message) {
@@ -1305,4 +1582,39 @@ private void assertLogResponse(LogPage logPage, UUID ruleId, AutomationRuleEvalu
13051582
.isEqualTo("The traceId '%s' was skipped for rule: '%s' and per the sampling rate '0.0'".formatted(
13061583
trace.id(), rule.getName()));
13071584
}
1585+
1586+
private void assertDisabledRuleLogResponse(LogPage logPage, UUID ruleId, AutomationRuleEvaluator<?> rule,
1587+
Trace trace) {
1588+
assertLogPage(logPage, 1);
1589+
1590+
assertThat(logPage.content())
1591+
.allSatisfy(log -> {
1592+
assertThat(log.timestamp()).isBetween(trace.createdAt(), Instant.now());
1593+
assertThat(log.ruleId()).isEqualTo(ruleId);
1594+
assertThat(log.level()).isEqualTo(LogLevel.INFO);
1595+
assertThat(log.markers()).isEqualTo(Map.of("trace_id", trace.id().toString()));
1596+
});
1597+
1598+
// Verify the message specifically mentions rule being disabled
1599+
assertThat(logPage.content().getFirst().message())
1600+
.isEqualTo("The traceId '%s' was skipped for rule: '%s' as the rule is disabled".formatted(
1601+
trace.id(), rule.getName()));
1602+
}
1603+
1604+
private void assertTraceThreadDisabledLogResponse(LogPage logPage, String threadModelId,
1605+
AutomationRuleEvaluator<?> rule,
1606+
Map<String, String> markers, Instant createdAt, String messageTemplate) {
1607+
assertLogPage(logPage, 1);
1608+
1609+
assertThat(logPage.content())
1610+
.allSatisfy(log -> {
1611+
assertThat(log.timestamp()).isBetween(createdAt, Instant.now());
1612+
assertThat(log.ruleId()).isEqualTo(rule.getId());
1613+
assertThat(log.level()).isEqualTo(LogLevel.INFO);
1614+
assertThat(log.markers()).isEqualTo(markers);
1615+
});
1616+
1617+
assertThat(logPage.content().getFirst().message())
1618+
.isEqualTo(messageTemplate.formatted(threadModelId, rule.getName()));
1619+
}
13081620
}

0 commit comments

Comments
 (0)