Skip to content

Conversation

emasab
Copy link
Contributor

@emasab emasab commented Jul 4, 2025

Closes #5135

@Copilot Copilot AI review requested due to automatic review settings July 4, 2025 13:20
@emasab emasab requested a review from a team as a code owner July 4, 2025 13:20
@confluent-cla-assistant
Copy link

🎉 All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a faster “quick” test runner configuration, updates plugin support detection in tests, fixes the OAuthBearer OIDC include guard, and extends the build‐configuration checks script to support both Make and CMake inside Docker.

  • Added a new quick brokerless test case in tests/CMakeLists.txt
  • Introduced runtime skip logic for unsupported plugin.library.paths tests in tests/0066-plugins.cpp
  • Switched #ifdef WITH_OAUTHBEARER_OIDC to #if WITH_OAUTHBEARER_OIDC in src/rdkafka_conf.c
  • Enhanced build-configurations-checks.sh to accept a build tool and optional Docker image, with corresponding updates in .semaphore/semaphore.yml

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/CMakeLists.txt Added RdKafkaTestBrokerLessQuick test target
tests/0066-plugins.cpp Wrapped plugin.path set in a skip condition for unsupported builds
src/rdkafka_conf.c Changed OIDC preprocessor to #if for proper zero/one testing
packaging/tools/build-configurations-checks.sh Generalized to handle make or cmake modes and Docker images
.semaphore/semaphore.yml Invokes the updated script with explicit tool/image combinations

@@ -69,6 +69,20 @@ static void do_test_plugin() {
NULL,
};

RdKafka::Conf *conf = RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL);
if (conf->set("plugin.library.paths",
"interceptor_test/interceptor_test_dummy", errstr) &&
Copy link
Preview

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

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

Rather than relying on the enum-to-bool conversion in if (conf->set(...)), explicitly compare the result to RdKafka::Conf::CONF_OK (or the appropriate error code) to make the intent clear.

Suggested change
"interceptor_test/interceptor_test_dummy", errstr) &&
"interceptor_test/interceptor_test_dummy", errstr) == RdKafka::Conf::CONF_OK &&

Copilot uses AI. Check for mistakes.

Comment on lines +75 to +81
errstr.find(
"Configuration property \"plugin.library.paths\" not supported") !=
std::string::npos) {
delete conf;
Test::Skip(
"Configuration property \"plugin.library.paths\" not supported in this "
"build\n");
Copy link
Preview

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

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

[nitpick] The same error message literal is repeated twice; consider extracting it into a named constant to avoid duplication and simplify future updates.

Suggested change
errstr.find(
"Configuration property \"plugin.library.paths\" not supported") !=
std::string::npos) {
delete conf;
Test::Skip(
"Configuration property \"plugin.library.paths\" not supported in this "
"build\n");
errstr.find(PLUGIN_PATHS_ERROR) != std::string::npos) {
delete conf;
Test::Skip(tostr() << PLUGIN_PATHS_ERROR << " in this build\n");

Copilot uses AI. Check for mistakes.

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.

librdkafka v2.11.0 can not build without curl
1 participant