Skip to content

Make ConfigUtils testable without illegal reflective access #14904

@mahesh989

Description

@mahesh989

Repository Version:
Apache Pinot (master)

Key Words:
ConfigUtils, testability, JDK17, illegal reflective access, unit tests

Description:

The issue happens because of illegal reflective access when testing ConfigUtils. Using reflection to change environment variables in tests doesn’t work well with JDK17. This update fixes the problem by adding an Environment interface, which makes it easier to pass environment variables in a way that can be tested without using illegal reflection.

Example:

public interface Environment {  
  String getVariable(String name);  
}

public static <T extends BaseJsonConfig> T applyConfigWithEnvVariables(Environment environment, T config) {  
  // Code implementation here  
}

Expected Behavior:
The updated ConfigUtils should make it easier to test without using reflection to access environment variables. Instead, environment variables should be passed directly through the Environment interface, which makes the code more flexible and easier to test.

Current Behavior:
In the old version, ConfigUtils used reflection to access environment variables, which caused issues in JDK17. This method triggered warnings about illegal reflective access during testing and when interacting with environment variables.

Steps to Reproduce:

  1. Try testing ConfigUtils in JDK17 with environment variables using reflection.
  2. You'll see illegal reflective access warnings during the tests.
  3. Apply the fix from the commit, which removes reflection and uses the Environment interface.
  4. Run the tests again and check that the issue is fixed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions