Skip to content

Commit 1b4b82f

Browse files
TylerJang27TylerJang27
authored
(Fix): Add new failure_information field to internal.bin (#1004)
We were previously capturing `status_output_message`, which was very limited compared to the JUnit parsing we were doing. Here's the new level of support: ```xml <testsuites> <testsuite name="suite_name" timestamp="2026-01-13T04:48:08" tests="0" file="file_path" time="204.494" failures="1"> <testcase name="case_name" time="0.000" classname="class_name"> <failure message="failure_message" type="type_name" system_out="system_out" system_err="system_err"> failure_text </failure> </testcase> </testsuite> </testsuites> ``` | JUnit Field | Historical JUnit Parsing Support | Internal.bin Historical Support | Internal.bin New Support | |--|--|--|--| | failure_message | failure_message | ❌ | failure_information.message | | failure_text | failure_text | (stored as) failure_message | failure_information.text | | system_out | system_out | ❌ | failure_information.system_out | | system_err | system_err | ❌ | failure_information.system_err | | type | ❌ | ❌ | ❌ | See [thread here](https://trunk-io.slack.com/archives/C08AEDGMZNH/p1769207730470679)
1 parent d4633bc commit 1b4b82f

File tree

6 files changed

+219
-18
lines changed

6 files changed

+219
-18
lines changed

‎context-js/tests/parse_and_validate.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
BranchClass,
2222
GitLabMergeRequestEventType,
2323
bin_parse,
24+
BindingsTestCaseStatusStatus,
2425
} from "../pkg/context_js";
2526

2627
// eslint-disable-next-line vitest/require-hook
@@ -392,6 +393,30 @@ describe("context-js", () => {
392393

393394
expect(rspecExpectationsSuite).toBeDefined();
394395
expect(rspecExpectationsSuite?.test_cases).toHaveLength(8);
396+
expect(rspecExpectationsSuite?.test_cases.at(0)?.status.status).toBe(
397+
BindingsTestCaseStatusStatus.Success,
398+
);
399+
expect(rspecExpectationsSuite?.test_cases.at(1)?.status.status).toBe(
400+
BindingsTestCaseStatusStatus.Success,
401+
);
402+
expect(rspecExpectationsSuite?.test_cases.at(2)?.status.status).toBe(
403+
BindingsTestCaseStatusStatus.Success,
404+
);
405+
expect(rspecExpectationsSuite?.test_cases.at(3)?.status.status).toBe(
406+
BindingsTestCaseStatusStatus.Success,
407+
);
408+
expect(rspecExpectationsSuite?.test_cases.at(4)?.status.status).toBe(
409+
BindingsTestCaseStatusStatus.Success,
410+
);
411+
expect(rspecExpectationsSuite?.test_cases.at(5)?.status.status).toBe(
412+
BindingsTestCaseStatusStatus.Success,
413+
);
414+
expect(rspecExpectationsSuite?.test_cases.at(6)?.status.status).toBe(
415+
BindingsTestCaseStatusStatus.Success,
416+
);
417+
expect(rspecExpectationsSuite?.test_cases.at(7)?.status.status).toBe(
418+
BindingsTestCaseStatusStatus.Success,
419+
);
395420
});
396421

397422
it("parses test_internal_bep.bin", () => {
@@ -416,6 +441,10 @@ describe("context-js", () => {
416441

417442
expect(testSuite).toBeDefined();
418443
expect(testSuite?.test_cases).toHaveLength(1);
444+
445+
expect(testSuite?.test_cases.at(0).status.status).toBe(
446+
BindingsTestCaseStatusStatus.Success,
447+
);
419448
expect(
420449
testSuite?.test_cases.at(0)?.js_extra().attempt_number,
421450
).toBeUndefined();
@@ -445,6 +474,9 @@ describe("context-js", () => {
445474

446475
expect(testSuite).toBeDefined();
447476
expect(testSuite?.test_cases).toHaveLength(1);
477+
expect(testSuite?.test_cases.at(0).status.status).toBe(
478+
BindingsTestCaseStatusStatus.Success,
479+
);
448480
expect(
449481
testSuite?.test_cases.at(0)?.js_extra().attempt_number,
450482
).toBeUndefined();

‎context/src/junit/bindings.rs‎

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,10 @@ impl From<TestResult> for BindingsReport {
204204
}
205205
}
206206

207+
fn non_empty_option(s: Option<&str>) -> Option<String> {
208+
s.filter(|s| !s.is_empty()).map(|s| s.to_string())
209+
}
210+
207211
impl From<TestCaseRun> for BindingsTestCase {
208212
fn from(
209213
TestCaseRun {
@@ -222,6 +226,7 @@ impl From<TestCaseRun> for BindingsTestCase {
222226
codeowners,
223227
attempt_index,
224228
line_number,
229+
test_output,
225230
}: TestCaseRun,
226231
) -> Self {
227232
let started_at = started_at.unwrap_or_default();
@@ -283,9 +288,16 @@ impl From<TestCaseRun> for BindingsTestCase {
283288
if typed_status == TestCaseRunStatus::Failure {
284289
Some(BindingsTestCaseStatusNonSuccess {
285290
kind: BindingsNonSuccessKind::Failure,
286-
message: Some(status_output_message.clone()),
291+
message: non_empty_option(
292+
test_output
293+
.as_ref()
294+
.map(|fi| fi.message.as_str())
295+
.or(Some(status_output_message.as_str())),
296+
),
287297
ty: None,
288-
description: None,
298+
description: non_empty_option(
299+
test_output.as_ref().map(|fi| fi.text.as_str()),
300+
),
289301
reruns: vec![],
290302
})
291303
} else {
@@ -295,17 +307,24 @@ impl From<TestCaseRun> for BindingsTestCase {
295307
skipped: {
296308
if typed_status == TestCaseRunStatus::Skipped {
297309
Some(BindingsTestCaseStatusSkipped {
298-
message: Some(status_output_message.clone()),
310+
message: non_empty_option(
311+
test_output
312+
.as_ref()
313+
.map(|fi| fi.message.as_str())
314+
.or(Some(status_output_message.as_str())),
315+
),
299316
ty: None,
300-
description: None,
317+
description: non_empty_option(
318+
test_output.as_ref().map(|fi| fi.text.as_str()),
319+
),
301320
})
302321
} else {
303322
None
304323
}
305324
},
306325
},
307-
system_err: None,
308-
system_out: None,
326+
system_err: non_empty_option(test_output.as_ref().map(|fi| fi.system_err.as_str())),
327+
system_out: non_empty_option(test_output.as_ref().map(|fi| fi.system_out.as_str())),
309328
extra,
310329
properties: vec![],
311330
}
@@ -1097,7 +1116,7 @@ mod tests {
10971116
AttemptNumber, CodeOwner, LineNumber, TestCaseRun, TestCaseRunStatus, TestResult,
10981117
};
10991118

1100-
use crate::junit::bindings::BindingsReport;
1119+
use crate::junit::bindings::{BindingsReport, BindingsTestCaseStatusStatus};
11011120
use crate::junit::parser::JunitParser;
11021121
use crate::junit::validator::{JunitValidationLevel, JunitValidationType};
11031122

@@ -1167,6 +1186,7 @@ mod tests {
11671186
#[test]
11681187
fn parse_test_report_to_bindings() {
11691188
use prost_wkt_types::Timestamp;
1189+
use proto::test_context::test_run::TestOutput;
11701190

11711191
use crate::junit::validator::validate;
11721192
let test_started_at = Timestamp {
@@ -1197,6 +1217,12 @@ mod tests {
11971217
finished_at: Some(test_finished_at.clone()),
11981218
status_output_message: "test_status_output_message".into(),
11991219
codeowners: vec![codeowner1],
1220+
test_output: Some(TestOutput {
1221+
message: "test_failure_message".into(),
1222+
text: "".into(),
1223+
system_out: "".into(),
1224+
system_err: "".into(),
1225+
}),
12001226
..Default::default()
12011227
};
12021228

@@ -1212,6 +1238,12 @@ mod tests {
12121238
started_at: Some(test_started_at.clone()),
12131239
finished_at: Some(test_finished_at),
12141240
status_output_message: "test_status_output_message".into(),
1241+
test_output: Some(TestOutput {
1242+
message: "".into(),
1243+
text: "test_status_output_message".into(),
1244+
system_out: "".into(),
1245+
system_err: "".into(),
1246+
}),
12151247
..Default::default()
12161248
};
12171249

@@ -1251,6 +1283,7 @@ mod tests {
12511283
assert_eq!(test_case1.time, Some(1000.0));
12521284
assert_eq!(test_case1.system_out, None);
12531285
assert_eq!(test_case1.system_err, None);
1286+
assert!(test_case1.status.success.is_some());
12541287
assert_eq!(test_case1.extra["id"], test1.id);
12551288
assert_eq!(test_case1.extra["file"], test1.file);
12561289
assert_eq!(
@@ -1284,6 +1317,14 @@ mod tests {
12841317
assert_eq!(test_case2.time, Some(1000.0));
12851318
assert_eq!(test_case2.system_out, None);
12861319
assert_eq!(test_case2.system_err, None);
1320+
assert_eq!(
1321+
test_case2.status.non_success.as_ref().unwrap().description,
1322+
Some(test2.test_output.clone().unwrap().text)
1323+
);
1324+
assert_eq!(
1325+
test_case2.status.non_success.as_ref().unwrap().message,
1326+
None
1327+
);
12871328
assert_eq!(test_case2.extra["id"], test2.id);
12881329
assert_eq!(test_case2.extra["file"], test2.file);
12891330
assert_eq!(test_case2.extra["line"], test2.line.to_string());
@@ -1392,7 +1433,6 @@ mod tests {
13921433
for (run_binding, report_binding) in
13931434
bindings_from_runs.iter().zip(bindings_from_reports.iter())
13941435
{
1395-
assert_eq!(run_binding.name, report_binding.name);
13961436
assert_eq!(run_binding.classname, report_binding.classname);
13971437
assert_eq!(run_binding.status.status, report_binding.status.status);
13981438
assert_eq!(run_binding.timestamp, report_binding.timestamp);
@@ -1403,6 +1443,16 @@ mod tests {
14031443
assert_eq!(run_binding.time, report_binding.time);
14041444
assert_eq!(run_binding.system_out, report_binding.system_out);
14051445
assert_eq!(run_binding.system_err, report_binding.system_err);
1446+
if run_binding.status.status == BindingsTestCaseStatusStatus::NonSuccess {
1447+
assert_eq!(
1448+
run_binding.status.non_success.as_ref().unwrap().description,
1449+
Some("Expected: <true> but was: <false>".into())
1450+
);
1451+
assert_eq!(
1452+
run_binding.status.non_success.as_ref().unwrap().message,
1453+
Some("Test failed".into())
1454+
);
1455+
}
14061456
// check that the properties match
14071457
for property in run_binding.properties.iter() {
14081458
if let Some(report_property) = report_binding
@@ -1455,6 +1505,7 @@ mod tests {
14551505
finished_at: Some(test_finished_at.clone()),
14561506
status_output_message: "".into(),
14571507
codeowners: vec![codeowner1.clone()],
1508+
test_output: None,
14581509
..Default::default()
14591510
};
14601511

@@ -1471,6 +1522,7 @@ mod tests {
14711522
finished_at: Some(test_finished_at.clone()),
14721523
status_output_message: "".into(),
14731524
codeowners: vec![codeowner2.clone()],
1525+
test_output: None,
14741526
..Default::default()
14751527
};
14761528

0 commit comments

Comments
 (0)