You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "Nachiket-Roy (via GitHub)" <gi...@apache.org> on 2025/12/27 12:15:32 UTC
[PR] docs : clarify unused test utility [datafusion]
Nachiket-Roy opened a new pull request, #19508:
URL: https://github.com/apache/datafusion/pull/19508
## 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?
- Added documentation explaining the purpose of the ctx_and_codec helper
- Explicitly marked the helper as intentionally unused to silence dead_code warnings
## Are these changes tested?
Yes.
- This change is documentation-only and does not affect runtime behavior.
- Existing test coverage remains unchanged and continues to pass.
## Are there any user-facing changes?
No.
This PR only affects internal test utilities and does not impact public APIs or user-facing behavior.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] docs : clarify unused test utility [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #19508:
URL: https://github.com/apache/datafusion/pull/19508#discussion_r2649130823
##########
datafusion/ffi/tests/utils/mod.rs:
##########
@@ -23,6 +23,11 @@ use datafusion_ffi::execution::FFI_TaskContextProvider;
use datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec;
use datafusion_proto::logical_plan::DefaultLogicalExtensionCodec;
+// Creates a default SessionContext and FFI Logical Extension Codec
+// for use in FFI integration tests.
+// This helper centralizes setup logic and is kept intentionally
+// for upcoming FFI test expansions.
+#[allow(dead_code)]
Review Comment:
```suggestion
#[expect(dead_code)]
```
- Thanks for this, after https://github.com/apache/datafusion/pull/19480 we'll need to change this to an expect
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] docs : clarify unused test utility [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #19508:
URL: https://github.com/apache/datafusion/pull/19508#discussion_r2649159257
##########
datafusion/ffi/tests/utils/mod.rs:
##########
@@ -23,6 +23,11 @@ use datafusion_ffi::execution::FFI_TaskContextProvider;
use datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec;
use datafusion_proto::logical_plan::DefaultLogicalExtensionCodec;
+// Creates a default SessionContext and FFI Logical Extension Codec
+// for use in FFI integration tests.
+// This helper centralizes setup logic and is kept intentionally
+// for upcoming FFI test expansions.
+#[allow(dead_code)]
Review 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?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] docs : clarify unused test utility [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19508:
URL: https://github.com/apache/datafusion/pull/19508#discussion_r2649560289
##########
datafusion/ffi/tests/utils/mod.rs:
##########
@@ -23,6 +23,11 @@ use datafusion_ffi::execution::FFI_TaskContextProvider;
use datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec;
use datafusion_proto::logical_plan::DefaultLogicalExtensionCodec;
+// Creates a default SessionContext and FFI Logical Extension Codec
+// for use in FFI integration tests.
+// This helper centralizes setup logic and is kept intentionally
+// for upcoming FFI test expansions.
+#[allow(dead_code)]
Review Comment:
Alright. Thanks.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] docs : clarify unused test utility [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19508:
URL: https://github.com/apache/datafusion/pull/19508#discussion_r2649198517
##########
datafusion/ffi/tests/utils/mod.rs:
##########
@@ -23,6 +23,11 @@ use datafusion_ffi::execution::FFI_TaskContextProvider;
use datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec;
use datafusion_proto::logical_plan::DefaultLogicalExtensionCodec;
+// Creates a default SessionContext and FFI Logical Extension Codec
+// for use in FFI integration tests.
+// This helper centralizes setup logic and is kept intentionally
+// for upcoming FFI test expansions.
+#[allow(dead_code)]
Review 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.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] docs : clarify unused test utility [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #19508:
URL: https://github.com/apache/datafusion/pull/19508#discussion_r2649544870
##########
datafusion/ffi/tests/utils/mod.rs:
##########
@@ -23,6 +23,11 @@ use datafusion_ffi::execution::FFI_TaskContextProvider;
use datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec;
use datafusion_proto::logical_plan::DefaultLogicalExtensionCodec;
+// Creates a default SessionContext and FFI Logical Extension Codec
+// for use in FFI integration tests.
+// This helper centralizes setup logic and is kept intentionally
+// for upcoming FFI test expansions.
+#[allow(dead_code)]
Review Comment:
```rust
#[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
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] docs : clarify unused test utility [datafusion]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan merged PR #19508:
URL: https://github.com/apache/datafusion/pull/19508
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] docs : clarify unused test utility [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19508:
URL: https://github.com/apache/datafusion/pull/19508#discussion_r2649145739
##########
datafusion/ffi/tests/utils/mod.rs:
##########
@@ -23,6 +23,11 @@ use datafusion_ffi::execution::FFI_TaskContextProvider;
use datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec;
use datafusion_proto::logical_plan::DefaultLogicalExtensionCodec;
+// Creates a default SessionContext and FFI Logical Extension Codec
+// for use in FFI integration tests.
+// This helper centralizes setup logic and is kept intentionally
+// for upcoming FFI test expansions.
+#[allow(dead_code)]
Review 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?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] docs : clarify unused test utility [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19508:
URL: https://github.com/apache/datafusion/pull/19508#discussion_r2649199256
##########
datafusion/ffi/tests/utils/mod.rs:
##########
@@ -23,6 +23,11 @@ use datafusion_ffi::execution::FFI_TaskContextProvider;
use datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec;
use datafusion_proto::logical_plan::DefaultLogicalExtensionCodec;
+// Creates a default SessionContext and FFI Logical Extension Codec
+// for use in FFI integration tests.
+// This helper centralizes setup logic and is kept intentionally
+// for upcoming FFI test expansions.
+#[allow(dead_code)]
Review 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.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] docs : clarify unused test utility [datafusion]
Posted by "Nachiket-Roy (via GitHub)" <gi...@apache.org>.
Nachiket-Roy commented on code in PR #19508:
URL: https://github.com/apache/datafusion/pull/19508#discussion_r2649138864
##########
datafusion/ffi/tests/utils/mod.rs:
##########
@@ -23,6 +23,11 @@ use datafusion_ffi::execution::FFI_TaskContextProvider;
use datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec;
use datafusion_proto::logical_plan::DefaultLogicalExtensionCodec;
+// Creates a default SessionContext and FFI Logical Extension Codec
+// for use in FFI integration tests.
+// This helper centralizes setup logic and is kept intentionally
+// for upcoming FFI test expansions.
+#[allow(dead_code)]
Review Comment:
Thanks for the review. Updated to #[expect(dead_code)].
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org