Skip to content

Remove number of metadata elements check in capi metadata tests - #4558

Open
mzegla wants to merge 3 commits into
mainfrom
unskip_capi_tests
Open

mzegla wants to merge 3 commits into
mainfrom
unskip_capi_tests

Conversation

@mzegla

@mzegla mzegla commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

🛠 Summary

JIRA/Issue if applicable.
Describe the changes.

🧪 Checklist

  • Unit tests added.
  • The documentation updated.
  • Change follows security best practices.
    ``

@mzegla
mzegla requested review from atobiszei and dtrawins and a lite review from Copilot September 14, 2026 09:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

An unused structured-binding element can fail builds compiled with -Wall -Werror.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates C API metadata tests to remove brittle exact-count assertions and re-enable skipped cases.

Changes:

  • Removes hard-coded metadata size checks.
  • Re-enables BasicDummy and BasicScalar tests.
  • Adds metadata-key diagnostic output.
  • Fixes required: an unused structured-binding element may fail -Wall -Werror builds.
File summaries
File Description
src/test/c_api_tests.cpp Updates metadata assertions, enables tests, and adds diagnostics; contains a build-blocking unused variable.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/test/c_api_tests.cpp Outdated
mzegla and others added 2 commits September 14, 2026 11:38
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.

2 participants