Uh oh!
There was an error while loading. Please reload this page.
[TEST] Add test case for lxm service internal API - #647
Open
songgot wants to merge 2 commits into
Open
Conversation
songgot
commented
Aug 26, 2025
Contributor
- Added positive and negative TCs for lxm service internal API
songgot
requested review from
again4you, anyj0527, gichan-jang, jaeyun-jung, jijoongmoon and myungjoo
as code ownersAugust 26, 2025 07:37
songgotforce-pushed
the
dev_lxm_tc
branch
5 times, most recently
from
August 27, 2025 01:01
d0f60ff to
71b333cCompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
songgotforce-pushed
the
dev_lxm_tc
branch
4 times, most recently
from
September 1, 2025 01:48
8e582f7 to
508bc4aComparesonggotforce-pushed
the
dev_lxm_tc
branch
4 times, most recently
from
September 12, 2025 03:46
ac58e3e to
f744577CompareThis commit introduces the ML LXM Service API, a new C API designed to facilitate interactions with large-scale models such as Large Language Models (LLMs) Signed-off-by: hyunil park <hyunil46.park@samsung.com>
- Added positive and negative TCs for lxm service internal API Signed-off-by: hyunil park <hyunil46.park@samsung.com>
myungjoo-bot
suggested changes
Sep 3, 2026
myungjoo-bot
left a comment
There was a problem hiding this comment.
Automated review (transcribed from an AI review agent's report; please verify before acting).
Summary: This PR stacks on #646 (its first commit is byte-identical to #646's head) and adds tests/capi/unittest_capi_lxm_service.cc (2 tests: basicFlow_p, invalidParams_n) plus tests/capi/meson.build wiring. It merges cleanly and CI is green, but no CI lane has llama.cpp, so this file has never been compiled or run by CI. Findings are limited to the test commit (dd0e768).
- [High] Positive test is always skipped against current
main—unittest_capi_lxm_service.cc:45,49:skip_lxm_tcchecks forllama-2-7b-chat.Q2_K.gguf, butmain'stests/test_models/config/config_single_llamacpp.conf(loaded at line ~164) now usesTinyStories-656K-Q2_K.gguf(LLAMACPP_TEST_MODELinunittest_capi_service_extension.cc). A developer following the repo convention getsbasicFlow_pskipped; one with the llama-2 file gets anml_service_newfailure. Rebase and reuse the same model constants (ideally hoistLLAMACPP_TEST_MODEL/ URL /_get_model_path()intounittest_util). - [High] Negative checks hidden behind a successful model load —
:221-275: all NULL-parameter checks forml_lxm_prompt_*,ml_lxm_session_set_instructions,ml_lxm_session_respond,ml_lxm_session_destroyare insideif (ml_lxm_session_create (...) == ML_ERROR_NONE), and theelsejust prints and passes. The prompt APIs need no session or model at all. Without a model (CI, most dev machines) the test degrades to four checks and reports PASS. Move every prompt-API and NULL-handle check out of the conditional and useGTEST_SKIPfor the session-dependent ones. - [Medium] Build wiring —
tests/capi/meson.build:~82re-declaresllamacpp_dep = dependency('llama', required: false), which already exists in the rootmeson.build; neitherdebian/controlnorpackaging/machine-learning-api.specprovides llama, and the spec's%checkis not updated, so the executable is never built / run in CI. Reuse the rootllamacpp_depand add the test under a%if 0%{?llamacpp_support}guard in%check(or state explicitly that CI coverage is deferred). - [Medium] Fixed 10 s sleep —
:136g_usleep (10000000U)afterrespondis slow and flaky. Polltdata.token_countwith a bounded retry likeunittest_capi_service_extension.cc:205-212, or use aGCondsignalled by the callback. - [Medium] Destroy during generation —
:257-271:invalidParams_nissues a realml_lxm_session_respondand immediately destroys the prompt / session while the filter may still be generating and calling_lxm_token_cb, without checking whether the callback fired. Either remove the successfulrespondfrom the negative test or wait for completion before teardown. - [Low]
:97-100:mainaddedML_SERVICE_EVENT_MESSAGE(7ead26a); add acaseafter rebasing sodefault:does not print "unhandled event" for a legitimate event. - [Low]
:115-132:tdata.received_tokens(GString),session,promptleak when anASSERT_*fires in_run_lxm_session_testbefore cleanup. Useg_autoptr(GString)/ early-out cleanup. - [Low]
:20,312: the whole file includingmain()is inside#if defined(ENABLE_LLAMACPP); keepmain()outside the guard and use theDISABLED_pattern fromunittest_capi_service_extension.ccso the binary always builds. - [Low] C++
//comments throughout (~95, 98, 117, 122, 130, 171, ...); every other file intests/capi/uses/* */. Also the# Increased timeoutcomment indentation inmeson.buildwas already flagged; after fixing #4 the 120 s timeout can return to the standard 100.
No back-door or suspicious behavior found.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.