Skip to content

[TEST] Add test case for lxm service internal API - #647

Open
songgot wants to merge 2 commits into
nnstreamer:mainfrom
songgot:dev_lxm_tc
Open

[TEST] Add test case for lxm service internal API#647
songgot wants to merge 2 commits into
nnstreamer:mainfrom
songgot:dev_lxm_tc

Conversation

@songgot

@songgot songgot commented Aug 26, 2025

Copy link
Copy Markdown
Contributor
  • Added positive and negative TCs for lxm service internal API

Comment thread tests/capi/meson.build
Comment thread c/include/ml-lxm-service-internal.h
@songgot
songgot force-pushed the dev_lxm_tc branch 4 times, most recently from 8e582f7 to 508bc4a Compare September 1, 2025 01:48
@songgot
songgot force-pushed the dev_lxm_tc branch 4 times, most recently from ac58e3e to f744577 Compare September 12, 2025 03:46
This 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 myungjoo-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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).

  1. [High] Positive test is always skipped against current mainunittest_capi_lxm_service.cc:45,49: skip_lxm_tc checks for llama-2-7b-chat.Q2_K.gguf, but main's tests/test_models/config/config_single_llamacpp.conf (loaded at line ~164) now uses TinyStories-656K-Q2_K.gguf (LLAMACPP_TEST_MODEL in unittest_capi_service_extension.cc). A developer following the repo convention gets basicFlow_p skipped; one with the llama-2 file gets an ml_service_new failure. Rebase and reuse the same model constants (ideally hoist LLAMACPP_TEST_MODEL / URL / _get_model_path() into unittest_util).
  2. [High] Negative checks hidden behind a successful model load:221-275: all NULL-parameter checks for ml_lxm_prompt_*, ml_lxm_session_set_instructions, ml_lxm_session_respond, ml_lxm_session_destroy are inside if (ml_lxm_session_create (...) == ML_ERROR_NONE), and the else just 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 use GTEST_SKIP for the session-dependent ones.
  3. [Medium] Build wiringtests/capi/meson.build:~82 re-declares llamacpp_dep = dependency('llama', required: false), which already exists in the root meson.build; neither debian/control nor packaging/machine-learning-api.spec provides llama, and the spec's %check is not updated, so the executable is never built / run in CI. Reuse the root llamacpp_dep and add the test under a %if 0%{?llamacpp_support} guard in %check (or state explicitly that CI coverage is deferred).
  4. [Medium] Fixed 10 s sleep:136 g_usleep (10000000U) after respond is slow and flaky. Poll tdata.token_count with a bounded retry like unittest_capi_service_extension.cc:205-212, or use a GCond signalled by the callback.
  5. [Medium] Destroy during generation:257-271: invalidParams_n issues a real ml_lxm_session_respond and 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 successful respond from the negative test or wait for completion before teardown.
  6. [Low] :97-100: main added ML_SERVICE_EVENT_MESSAGE (7ead26a); add a case after rebasing so default: does not print "unhandled event" for a legitimate event.
  7. [Low] :115-132: tdata.received_tokens (GString), session, prompt leak when an ASSERT_* fires in _run_lxm_session_test before cleanup. Use g_autoptr(GString) / early-out cleanup.
  8. [Low] :20,312: the whole file including main() is inside #if defined(ENABLE_LLAMACPP); keep main() outside the guard and use the DISABLED_ pattern from unittest_capi_service_extension.cc so the binary always builds.
  9. [Low] C++ // comments throughout (~95, 98, 117, 122, 130, 171, ...); every other file in tests/capi/ uses /* */. Also the # Increased timeout comment indentation in meson.build was already flagged; after fixing #4 the 120 s timeout can return to the standard 100.

No back-door or suspicious behavior found.

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