DevShiba commented on PR #24218:
URL: https://github.com/apache/datafusion/pull/24218#issuecomment-5355533730

   @kosiew Good catches on both — pushed fixes.
   
   **SemVer/ABI**: reworked `create_exec_with_byte_metrics` as its own 
top-level exported symbol 
(`datafusion_ffi_test_create_exec_with_byte_metrics`), loaded via a new 
`get_byte_metrics_exec()` in `tests/utils.rs` that mirrors exactly how 
`get_module()` already loads `datafusion_ffi_get_module` (separate 
`libloading::Library`, `lib.get(...)`, leak to keep it loaded). 
`ForeignLibraryModule` no longer has the new field — confirmed via `git diff` 
against the pre-fix commit that its fields are now byte-for-byte identical to 
before this PR touched it.
   
   **Discriminant coverage**: the test now `match`es `metric.value()` against 
`MetricValue::BytesCount { name, count }` / `MetricValue::BytesGauge { name, 
gauge }` explicitly and asserts both the name and the numeric value, on top of 
(not instead of) the existing Display-string assertions.
   
   Verified locally before pushing: full `datafusion-ffi --features 
integration-tests` suite, the exact three commands the previously-failing CI 
jobs run, the exact `ci/scripts/rust_clippy.sh` script, and `cargo fmt --check` 
— all clean.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to