Skip to content

Catch warnings in FFI unit tests#1410

Merged
timsaucer merged 2 commits intoapache:mainfrom
timsaucer:test/catch-log-warnings
Mar 7, 2026
Merged

Catch warnings in FFI unit tests#1410
timsaucer merged 2 commits intoapache:mainfrom
timsaucer:test/catch-log-warnings

Conversation

@timsaucer
Copy link
Member

@timsaucer timsaucer commented Mar 5, 2026

Which issue does this PR close?

None, but related to apache/datafusion#20704

Rationale for this change

Right now we have unit tests that are generating warnings. These could have helped us catch the issue above before it got released. Instead the warnings were silently ignored. With this PR we will prevent future warnings from getting through CI.

What changes are included in this PR?

Initialize logging in example crate.
Add check for warnings in unit tests.

Are there any user-facing changes?

No, CI only.

@timsaucer
Copy link
Member Author

@timsaucer timsaucer force-pushed the test/catch-log-warnings branch from 89d5d15 to 8442412 Compare March 6, 2026 18:25
Copy link
Member

@davisp davisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1


#[pymodule]
fn datafusion_ffi_example(m: &Bound<'_, PyModule>) -> PyResult<()> {
pyo3_log::init();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat!

Copy link
Contributor

@kevinjqliu kevinjqliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timsaucer timsaucer merged commit f914fc8 into apache:main Mar 7, 2026
19 checks passed
@timsaucer timsaucer deleted the test/catch-log-warnings branch March 7, 2026 02:13
@timsaucer
Copy link
Member Author

Thank you @davisp and @kevinjqliu !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants