Skip to content

Conversation

@MWSestabro
Copy link
Member

Added some test cases addressing some of the larger code coverage gaps in the codebase.

@MWSestabro MWSestabro requested a review from duncanpo October 8, 2025 17:12
@MWSestabro MWSestabro self-assigned this Oct 8, 2025
Copy link
Member

@duncanpo duncanpo left a comment

Choose a reason for hiding this comment

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

Looks good. Just a couple of very minor issues that you can quickly fix

test/tformat.m Outdated

methods (TestMethodSetup)
function setup(testCase)
config = fullfile(fileparts(mfilename("fullpath")), "config", "otelcol_config.yml");
Copy link
Member

Choose a reason for hiding this comment

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

This is the default configuration file. You don't need to specify it here. Simply call commonSetup without passing in the configuration and you will get the default.


value = 30;
result = opentelemetry.metrics.ObservableResult;
result = result.observe(value, dictionary({"Level1"}, {"D"},{"Level2"},{"E"})); No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Do you really need cell arrays of strings in the dictionary? Can you avoid using cell arrays:
dictionary("Level1", "D", "Level2", "E")


methods (TestMethodSetup)
function setup(testCase)
config = fullfile(fileparts(mfilename("fullpath")), "config", "otelcol_config.yml");
Copy link
Member

Choose a reason for hiding this comment

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

No need to supply the default configuration.


methods (TestMethodSetup)
function setup(testCase)
config = fullfile(fileparts(mfilename("fullpath")), "config", "otelcol_config.yml");
Copy link
Member

Choose a reason for hiding this comment

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

No need to supply default configurations.

test/ttimeout.m Outdated

methods (TestMethodSetup)
function setup(testCase)
config = fullfile(fileparts(mfilename("fullpath")), "config", "otelcol_config.yml");
Copy link
Member

Choose a reason for hiding this comment

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

default config not needed.


methods (TestMethodSetup)
function setup(testCase)
config = fullfile(fileparts(mfilename("fullpath")), "config", "otelcol_config.yml");
Copy link
Member

Choose a reason for hiding this comment

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

default configuration not needed

@duncanpo duncanpo merged commit 174b98a into main Oct 9, 2025
5 checks passed
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.

3 participants