Repository navigation
docs : clarify unused test utility - #19508
Conversation
| // for use in FFI integration tests. | ||
| // This helper centralizes setup logic and is kept intentionally | ||
| // for upcoming FFI test expansions. | ||
| #[allow(dead_code)] |
There was a problem hiding this comment.
| #[allow(dead_code)] | |
| #[expect(dead_code)] |
- Thanks for this, after chore: enforce clippy::allow_attributes for datafusion-ffi crate #19480 we'll need to change this to an expect
There was a problem hiding this comment.
Thanks for the review. Updated to #[expect(dead_code)].
There was a problem hiding this comment.
The lint expectation is currently unfulfilled so #[expect(dead_code)] fails CI.
Should I revert this back to #[allow(dead_code)] for now and switch to #[expect] once the lint actually triggers?
There was a problem hiding this comment.
We should figure out why it isn't fulfilled in that case; it doesn't make sense that allow suppresses a lint that isn't fulfilled if we replace it with expect unless its a bug in the linter. Perhaps check if it occurs when a certain feature is enabled/disabled?
There was a problem hiding this comment.
The helper is used by FFI integration tests, but its usage depends on which tests and features are compiled. In some configurations, the compiler does not see any references to it and therefore does not emit a dead_code lint. In those cases, #[expect(dead_code)] fails because the lint is not triggered, while #[allow(dead_code)] still works since it does not require the lint to be emitted.
There was a problem hiding this comment.
I’ve switched this back to #[allow(dead_code)] for now. Since the dead_code lint isn’t consistently emitted across all test / feature configurations, using #[expect(dead_code)] causes failures when the lint doesn’t trigger. allow avoids that while still documenting the intent.
There was a problem hiding this comment.
#[cfg_attr(not(feature = "integration-tests"), expect(dead_code))]This should work; it conditionally expects this as dead code if not using integration-tests feature
There was a problem hiding this comment.
Alright. Thanks.
Which issue does this PR close?
Closes N/A (no existing issue)
This change addresses a compiler warning observed during local test runs and does not correspond to a tracked bug or feature request.
Rationale for this change
During local test runs, the helper function ctx_and_codec in datafusion/ffi/tests/utils triggered a dead_code warning.
While the function is currently unused, it encapsulates non-trivial setup logic for FFI integration tests and is expected to be reused as FFI test coverage expands.
This change documents the intent of the helper and explicitly marks it as intentionally retained scaffolding, improving code clarity.
What changes are included in this PR?
Are these changes tested?
Yes.
Are there any user-facing changes?
No.
This PR only affects internal test utilities and does not impact public APIs or user-facing behavior.