Add comprehensive tests for 11 modules with AAA pattern comments by Copilot · Pull Request #2408 · microsoft/Olive

Copilot · 2026-04-10T03:33:27Z

…onstants, logging, config_utils, metric_config, metric_result, search_parameter, search_point, search_sample, data registry, and telemetry utils Agent-Logs-Url: https://github.com/microsoft/Olive/sessions/656d3838-fc06-4d40-8a11-b75da7f23724 Co-authored-by: xiaoyu-work <85524621+xiaoyu-work@users.noreply.github.com>

Agent-Logs-Url: https://github.com/microsoft/Olive/sessions/656d3838-fc06-4d40-8a11-b75da7f23724 Co-authored-by: xiaoyu-work <85524621+xiaoyu-work@users.noreply.github.com>

…ethods Agent-Logs-Url: https://github.com/microsoft/Olive/sessions/6dac7c0c-a4f5-4649-9a9a-1d9237f27a3d Co-authored-by: xiaoyu-work <85524621+xiaoyu-work@users.noreply.github.com>

…xists Agent-Logs-Url: https://github.com/microsoft/Olive/sessions/3dc88a98-09ac-43c9-86b5-b2eddc3e34ae Co-authored-by: xiaoyu-work <85524621+xiaoyu-work@users.noreply.github.com>

test/telemetry/test_telemetry_utils.py

Agent-Logs-Url: https://github.com/microsoft/Olive/sessions/4d88a644-eb92-4192-8494-b25f650bc10d Co-authored-by: xiaoyu-work <85524621+xiaoyu-work@users.noreply.github.com>

Copilot

Pull request overview

This PR adds new unit tests across several Olive modules, aiming to improve coverage and adopt a consistent AAA-style structure in test methods.

Changes:

Added new pytest suites for logging, exceptions, constants, telemetry utilities, search primitives, evaluator metric types, data registry, and config utilities.
Introduced parameterized tests for validation and mapping behaviors (for example metric goals, logger levels, precision to bits).
Added telemetry and search serialization roundtrip tests (to_json and from_json).

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
test/test_logging.py	Adds coverage for logger creation, verbosity setters, env-based configuration, and file logging.
test/test_exception.py	Adds basic tests for Olive exception hierarchy and expected exception tuples.
test/test_constants.py	Adds tests for constants and enum behaviors, including mapping helpers.
test/telemetry/test_telemetry_utils.py	Adds tests for telemetry path resolution, exception formatting, and base64 cache line encode/decode.
test/telemetry/init.py	Adds package init for telemetry tests.
test/search/test_search_sample.py	Adds tests for `SearchSample` config extraction, JSON roundtrip, and repr.
test/search/test_search_point.py	Adds tests for `SearchPoint` construction, equality, validity checks, and JSON roundtrip.
test/search/test_search_parameter.py	Adds tests for search parameter classes and JSON factory logic.
test/evaluator/test_metric_result.py	Adds tests for metric result containers and flattening helpers.
test/evaluator/test_metric_config.py	Adds tests for metric config defaults and `MetricGoal` validation logic.
test/data_container/test_registry.py	Adds tests for data component registration and retrieval behavior.
test/common/test_config_utils.py	Adds broad coverage for config serialization, loading, validation, and dynamic config class creation.

Copilot · 2026-04-10T03:48:50Z

test/test_constants.py

+        assert result == expected
+
+
+class TestPrecision:
+    def test_precision_all_members(self):
+        # setup
+        expected_count = 14
+
+        # execute
+        result = len(Precision)
+
+        # assert
+        assert result == expected_count
+
+
+class TestQuantAlgorithm:
+    def test_quant_algorithm_case_insensitive(self):


The PR description says to remove enum/constant value tests that only assert Enum.MEMBER == "literal" without exercising behavior, but this file still includes several of those (for example test_framework_values, test_framework_str, and similar value/str tests below). Please remove these nonsensical value-only assertions and keep the behavior-focused tests (case-insensitive parsing, __members__ completeness, and precision_bits_from_precision mapping).

Copilot · 2026-04-10T03:48:51Z

test/search/test_search_point.py

+        values = OrderedDict(
+            {
+                "pass1": OrderedDict(
+                    {
+                        "param1": SpecialParamValue.INVALID,
+                    }
+                )
+            }
+        )
+        point = SearchPoint(index=0, values=values)
+


SearchPoint.values is defined/produced as dict[str, tuple[int, Any]] (for example via SearchSpace.get_sample_point_values), but this test builds values as nested OrderedDict without the (index, value) tuples. That means the test is not validating behavior under the real data structure and can mask issues in SearchPoint.is_valid(). Please construct values using the same tuple shape as production code (and then assert validity accordingly).

Copilot · 2026-04-10T03:48:51Z

test/test_logging.py

+        # execute
+        with patch.dict("os.environ", {}, clear=True):
+            set_verbosity_from_env()
+
+        # assert (no exception raised)
+


This test currently has no assertions, so it will not catch regressions in the default behavior of set_verbosity_from_env() (which should set the logger level based on the default "INFO"). Please assert the expected resulting logger level (and restore the prior level afterward to avoid cross-test pollution).

Suggested change

# execute

with patch.dict("os.environ", {}, clear=True):

set_verbosity_from_env()

# assert (no exception raised)

# setup

previous_level = get_olive_logger().level

try:

# execute

with patch.dict("os.environ", {}, clear=True):

set_verbosity_from_env()

# assert

assert get_olive_logger().level == logging.INFO

finally:

# cleanup

set_verbosity(previous_level)

Copilot · 2026-04-10T03:48:51Z

test/test_logging.py

+class TestSetVerbosity:
+    def test_set_verbosity_info(self):
+        # execute
+        set_verbosity_info()
+
+        # assert
+        assert get_olive_logger().level == logging.INFO
+
+    def test_set_verbosity_warning(self):
+        # execute
+        set_verbosity_warning()
+
+        # assert
+        assert get_olive_logger().level == logging.WARNING
+
+    def test_set_verbosity_debug(self):
+        # execute
+        set_verbosity_debug()
+
+        # assert
+        assert get_olive_logger().level == logging.DEBUG
+
+    def test_set_verbosity_error(self):
+        # execute
+        set_verbosity_error()
+
+        # assert
+        assert get_olive_logger().level == logging.ERROR
+
+    def test_set_verbosity_critical(self):
+        # execute
+        set_verbosity_critical()
+
+        # assert
+        assert get_olive_logger().level == logging.CRITICAL
+
+    def test_set_verbosity_custom_level(self):
+        # execute
+        set_verbosity(logging.WARNING)
+
+        # assert
+        assert get_olive_logger().level == logging.WARNING


These tests mutate the global olive logger level and do not restore it, which can make other tests order-dependent. Please capture the original logger level (and ideally handlers) and restore them in a fixture or teardown so this module does not leak global logging state.

Copilot · 2026-04-10T03:48:51Z

test/test_logging.py

+
+        # execute
+        enable_filelog(1, str(tmp_path), workflow_id)
+
+        # assert
+        logger = get_olive_logger()
+        log_file_path = tmp_path / f"{workflow_id}.log"
+        file_handlers = [h for h in logger.handlers if isinstance(h, logging.FileHandler)]
+        assert len(file_handlers) > 0
+
+        # cleanup
+        for h in file_handlers:
+            if Path(h.baseFilename) == log_file_path.resolve():
+                logger.removeHandler(h)
+                h.close()


test_enable_filelog_creates_handler only asserts that some FileHandler exists on the global logger. This can become a false positive if any prior test added a file handler. Please assert that a handler was added for the expected workflow_id path (and consider verifying the file is created), and restore the logger handler list to its prior state to prevent leaks.

Suggested change

# execute

enable_filelog(1, str(tmp_path), workflow_id)

# assert

logger = get_olive_logger()

log_file_path = tmp_path / f"{workflow_id}.log"

file_handlers = [h for h in logger.handlers if isinstance(h, logging.FileHandler)]

assert len(file_handlers) > 0

# cleanup

for h in file_handlers:

if Path(h.baseFilename) == log_file_path.resolve():

logger.removeHandler(h)

h.close()

logger = get_olive_logger()

original_handlers = list(logger.handlers)

original_handler_ids = {id(handler) for handler in original_handlers}

log_file_path = (tmp_path / f"{workflow_id}.log").resolve()

try:

# execute

enable_filelog(1, str(tmp_path), workflow_id)

# assert

new_handlers = [handler for handler in logger.handlers if id(handler) not in original_handler_ids]

matching_handlers = [

handler

for handler in new_handlers

if isinstance(handler, logging.FileHandler) and Path(handler.baseFilename).resolve() == log_file_path

]

assert matching_handlers, f"Expected a FileHandler for {log_file_path}, but none was added."

assert log_file_path.exists()

finally:

# cleanup

for handler in [handler for handler in logger.handlers if id(handler) not in original_handler_ids]:

logger.removeHandler(handler)

handler.close()

Copilot · 2026-04-10T03:48:52Z

test/evaluator/test_metric_config.py

+    def test_unknown_metric_type(self):
+        # execute
+        cls = get_user_config_class("latency")
+        instance = cls()
+
+        # assert
+        assert hasattr(instance, "user_script")


The test name test_unknown_metric_type is misleading because it calls get_user_config_class("latency"), which is a known metric type. Please rename the test to reflect what is actually being validated (for example, that non-"custom" metric types still include the common user-config fields).

test/common/test_config_utils.py

+
+    def test_invalid_value_returns_none(self):
+        # execute
+        result = ParamCategory._missing_("nonexistent")


…prove assertions, rename test Agent-Logs-Url: https://github.com/microsoft/Olive/sessions/6e93cade-0aec-4b0e-b700-2063624d8f7d Co-authored-by: xiaoyu-work <85524621+xiaoyu-work@users.noreply.github.com>

Copilot AI and others added 3 commits April 9, 2026 23:58

Improve comment clarity in test_search_point.py per code review

fc72c73

Agent-Logs-Url: https://github.com/microsoft/Olive/sessions/656d3838-fc06-4d40-8a11-b75da7f23724 Co-authored-by: xiaoyu-work <85524621+xiaoyu-work@users.noreply.github.com>

Add # setup / # execute / # assert section comments to all new test m…

142c767

…ethods Agent-Logs-Url: https://github.com/microsoft/Olive/sessions/6dac7c0c-a4f5-4649-9a9a-1d9237f27a3d Co-authored-by: xiaoyu-work <85524621+xiaoyu-work@users.noreply.github.com>

Copilot AI assigned Copilot and xiaoyu-work Apr 10, 2026

Copilot created this pull request from a session on behalf of xiaoyu-work April 10, 2026 03:33 View session

Copilot started work on behalf of xiaoyu-work April 10, 2026 03:34 View session

Remove empty # setup comments from test methods where no setup code e…

abdc8ad

…xists Agent-Logs-Url: https://github.com/microsoft/Olive/sessions/3dc88a98-09ac-43c9-86b5-b2eddc3e34ae Co-authored-by: xiaoyu-work <85524621+xiaoyu-work@users.noreply.github.com>

Copilot AI requested a review from xiaoyu-work April 10, 2026 03:39

Copilot finished work on behalf of xiaoyu-work April 10, 2026 03:39

Copilot started work on behalf of xiaoyu-work April 10, 2026 03:40 View session

xiaoyu-work marked this pull request as ready for review April 10, 2026 03:45

Copilot AI review requested due to automatic review settings April 10, 2026 03:45

Copilot started reviewing on behalf of xiaoyu-work April 10, 2026 03:46 View session

github-advanced-security bot found potential problems Apr 10, 2026

View reviewed changes

test/telemetry/test_telemetry_utils.py Fixed Show fixed Hide fixed

test/telemetry/test_telemetry_utils.py Fixed Show fixed Hide fixed

Remove nonsensical enum/constant value tests from test_constants.py

5b0187a

Agent-Logs-Url: https://github.com/microsoft/Olive/sessions/4d88a644-eb92-4192-8494-b25f650bc10d Co-authored-by: xiaoyu-work <85524621+xiaoyu-work@users.noreply.github.com>

Copilot AI reviewed Apr 10, 2026

View reviewed changes

Copilot finished work on behalf of xiaoyu-work April 10, 2026 03:49

Copilot started work on behalf of xiaoyu-work April 10, 2026 04:01 View session

github-advanced-security bot found potential problems Apr 10, 2026

View reviewed changes

test/common/test_config_utils.py

def test_invalid_value_returns_none(self):

# execute

result = ParamCategory._missing_("nonexistent")

Address PR review comments: fix CodeQL issues, add logger fixture, im…

cb03be7

…prove assertions, rename test Agent-Logs-Url: https://github.com/microsoft/Olive/sessions/6e93cade-0aec-4b0e-b700-2063624d8f7d Co-authored-by: xiaoyu-work <85524621+xiaoyu-work@users.noreply.github.com>

Copilot finished work on behalf of xiaoyu-work April 10, 2026 04:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add comprehensive tests for 11 modules with AAA pattern comments#2408

Add comprehensive tests for 11 modules with AAA pattern comments#2408
Copilot wants to merge 6 commits intomainfrom
copilot/analyze-test-coverage

Copilot AI commented Apr 10, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Copilot AI commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Apr 10, 2026 •

edited

Loading