-
Notifications
You must be signed in to change notification settings - Fork 6
(Chore): Add bazel started_at, etc. to internal.bin #1006
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(Chore): Add bazel started_at, etc. to internal.bin #1006
Conversation
|
😎 Merged successfully - details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unchanged, just moved
Only minor change is pub(crate) on the extra field
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unchanged, just moved
All the testing that was in bindings.rs got relocated here for now
| struct TimestampWrapper { | ||
| datetime: chrono::DateTime<chrono::Utc>, | ||
| timestamp: i64, | ||
| timestamp_micros: i64, | ||
| } | ||
|
|
||
| impl From<prost_wkt_types::Timestamp> for TimestampWrapper { | ||
| fn from(value: prost_wkt_types::Timestamp) -> Self { | ||
| let datetime = chrono::DateTime::from(value.clone()); | ||
| TimestampWrapper { | ||
| datetime, | ||
| timestamp: datetime.timestamp(), | ||
| timestamp_micros: datetime.timestamp_micros(), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new in order to commonize some of the date parsing
| let started_at = started_at.unwrap_or_default(); | ||
| let started_at_wrapper = TimestampWrapper::from(started_at); | ||
| let time = (chrono::DateTime::from(finished_at.unwrap_or_default()) | ||
| - started_at_wrapper.datetime) | ||
| .to_std() | ||
| .unwrap_or_default(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tweaked/commonized
| bazel_run_information: match test_runner_information { | ||
| Some(proto::test_context::test_run::test_case_run::TestRunnerInformation::BazelRunInformation( | ||
| bazel_run_information, | ||
| )) => { | ||
| let started_at_wrapper = TimestampWrapper::from(bazel_run_information.started_at.unwrap_or_default()); | ||
| let finished_at_wrapper = TimestampWrapper::from(bazel_run_information.finished_at.unwrap_or_default()); | ||
|
|
||
| Some(BindingsBazelRunInformation { | ||
| label: bazel_run_information.label, | ||
| attempt_number: bazel_run_information.attempt_number, | ||
| started_at: Some(started_at_wrapper.timestamp), | ||
| started_at_micros: Some(started_at_wrapper.timestamp_micros), | ||
| finished_at: Some(finished_at_wrapper.timestamp), | ||
| finished_at_micros: Some(finished_at_wrapper.timestamp_micros), | ||
| }) | ||
| }, | ||
| _ => None, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new
| pub struct BindingsBazelRunInformation { | ||
| pub label: String, | ||
| pub attempt_number: i32, | ||
| pub started_at: Option<i64>, | ||
| pub started_at_micros: Option<i64>, | ||
| pub finished_at: Option<i64>, | ||
| pub finished_at_micros: Option<i64>, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new. Please advise on the typings for this, not sure what best practice is for timestamp stuff. I mainly was just cargo culting the prior timestamp stuff in each context
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1006 +/- ##
==========================================
+ Coverage 81.19% 81.53% +0.34%
==========================================
Files 66 69 +3
Lines 14326 14468 +142
==========================================
+ Hits 11632 11797 +165
+ Misses 2694 2671 -23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: max-trunk <max@trunk.io> Signed-off-by: Tyler Jang <tyler@trunk.io>
Adds
started_at,finished_at,attempt_number, andlabelas test run-level information in the internal.bin file. We need the bazel information to be serialized because it is sometimes more accurate than the times reported by JUnit reporters. Leaving the conversion to be handled during ingestion, but it should betestCase.bazel_run_information?.started_at ?? testCase.started_at.More info in thread
For a JUnit.xml like:
we end up with an internal.bin like:
{ "test_results": [ { "test_case_runs": [ { "id": "", "name": "case_name", "classname": "class_name", "file": "file_path", "parent_name": "suite_name", "line": 25, "status": 1, "attempt_number": 0, "started_at": "2026-01-13T04:48:08.000Z", "finished_at": "2026-01-13T04:50:23.916Z", "status_output_message": "", "is_quarantined": false, "codeowners": [], "attempt_index": null, "line_number": { "number": 25 }, "test_output": null, "test_runner_information": { "BazelRunInformation": { "label": "//bazel_label", "attempt_number": 0, "started_at": "2026-01-13T04:50:22.996Z", "finished_at": "2026-01-13T04:50:24.011Z" } } }, ... ]}]}I also broke up
bindings.rsbecause it was painfully long. I've called out the specific changes that I've made in that directory.