Skip to content

tests: add model resolution test on synthetic repo listings - #26172

Merged
ServeurpersoCom merged 22 commits into
ggml-org:masterfrom
ServeurpersoCom:test-model-resolution
Aug 3, 2026
Merged

tests: add model resolution test on synthetic repo listings#26172
ServeurpersoCom merged 22 commits into
ggml-org:masterfrom
ServeurpersoCom:test-model-resolution

Conversation

@ServeurpersoCom

Copy link
Copy Markdown
Contributor

Overview

Adding the synthetic repo tests we discussed on #26165, download.cpp and arg.cpp are included inside a namespace with hf_cache monkey patched to serve hardcoded listings mimicking real vendor conventions, so the resolution and the model handler assembly run end-to-end through the real CLI parsing on permuted listings, no code modified and no network needed, dspark will just add its keyword, a fixture line and a table line

The test harness is table-driven so new layouts or resolution rules can be added with a fixture line and a table line

Additional information

Follow-up of #26165

Requirements

@github-actions github-actions Bot added the testing Everything test related label Jul 27, 2026
@ServeurpersoCom

Copy link
Copy Markdown
Contributor Author

Ouch, I'm fixing CI

@ServeurpersoCom
ServeurpersoCom force-pushed the test-model-resolution branch from d22f91f to fc3d2da Compare July 28, 2026 16:14

@ggerganov ggerganov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems quite a bit hacky and difficult for human to follow. But it's probably better than nothing. Let's have @ngxson take a look too.

@ServeurpersoCom

ServeurpersoCom commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Yes, it's just an idea that's easy to refactor using AI, but we could also create a real repo with dummy mini-files, though that would involve a small internet request.

I actually started with the other approach, exposing the resolution and the task assembly as functions taking the listing as input, which gives a trivial readable test at the cost of a small API addition. The real solution would be to refactor the file resolution into a clean reusable unit, but I am not sure it is worth it.

Current approach is that it runs the real production code path end-to-end, real CLI parsing and real handler init, so it will warn if we break actual model downloads, while exposed functions would only test what we choose to expose.

@ggerganov

Copy link
Copy Markdown
Member

Yes, let's see if this approach would work well enough. We can reconsider later if we still encounter problems.

@ServeurpersoCom

Copy link
Copy Markdown
Contributor Author

Yes, it acts as a canary that will break on the first refactor, which means the refactor will have to take testability into account, and this hack can then be removed.

@ServeurpersoCom

Copy link
Copy Markdown
Contributor Author

Refactor way look like :

Testable heuristic 33bf0c6
Test unit c63f955

Comment thread tests/test-model-resolution.cpp Outdated
Comment on lines +120 to +129
std::pair<long, std::vector<char>> common_remote_get_content(const std::string & url, const common_remote_params & params);
std::pair<std::string, std::string> common_download_split_repo_tag(const std::string & hf_repo_with_tag);
void common_download_run_tasks(const std::vector<common_download_task> & tasks);
std::vector<std::string> common_download_get_all_parts(const std::string & url);
std::vector<common_cached_model_info> common_list_cached_models();
int common_download_file_single(const std::string & url, const std::string & path, const common_download_opts & opts = {}, bool skip_etag = false);
std::string common_docker_resolve_model(const std::string & docker);
bool common_download_remove(const std::string & hf_repo_with_tag);
common_download_hf_plan common_download_get_hf_plan(const common_params_model & model, const common_download_opts & opts);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this test case is useful, but we should find a way to avoid all of the boilerplate on the top of this file

I assume this boilerplate is to avoid making network calls. wondering if we can just stub the httplib:Client by making a derived class from it. will quickly test if that's even possible or not.

btw, can you confirm if there are any other reasons why we need this long boilerplate?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I just made the cleaner alternative PTAL #26247

@ServeurpersoCom ServeurpersoCom Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

About the boilerplate: the network avoidance is the small part (the 20 line fake hf_cache, the natural seam, httplib::Client would not intercept the cached listings). The rest is the price of compiling namespaced copies of the sources: ADL ambiguity with the global declarations forces the rename macros and the verbatim redeclarations for the fatal warnings CI, plus the unused pragmas, the winsock ordering and the explicit cpp-httplib link under lld-link.

But given that auto downloading is an essential component of the user experience, and that the other modalities (vocoder etc...) will bring additional cases, I think the refactor is worth it.

@ngxson

ngxson commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Ok so let's wrap httplib::Client around a new struct common_http_client, and provide a derived stub class at test time. I partially done the work here, feel free to pick it up: master...xsn/demo_common_http_client

@ServeurpersoCom
ServeurpersoCom requested a review from a team as a code owner July 29, 2026 08:20

@ngxson ngxson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

much cleaner now

Comment on lines +32 to +47
#define REQUIRE(x) do { \
if (!(x)) { \
fprintf(stderr, "%s:%d: [%s] REQUIRE(%s) failed\n", \
__FILE__, __LINE__, g_context.c_str(), #x); \
std::abort(); \
} \
} while (0)

#define REQUIRE_EQ(actual, expected) do { \
if (!((actual) == (expected))) { \
fprintf(stderr, "%s:%d: [%s] REQUIRE_EQ(%s, %s) failed\n actual: '%s'\n expected: '%s'\n", \
__FILE__, __LINE__, g_context.c_str(), #actual, #expected, \
std::string(actual).c_str(), std::string(expected).c_str()); \
std::abort(); \
} \
} while (0)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

not sure if we should reuse the testing.h framework here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good fit I think, the continue-on-failure would show the whole table on a red run instead of the first abort. I would do it as a follow-up though, the assertions guard some control flow (parse before init) so it is a real rework of the test, not a swap.

Comment thread common/http.h Outdated
common_http_client_ptr common_http_client_create(const std::string & url);

// substitute the client creation, e.g. with a stub (ONLY for testing)
void common_http_client_set_factory(common_http_client_ptr (*factory)(const std::string & url));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

cosmetic: maybe typedef the callback into a shorter naming common_http_client_factory_fn

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread tests/test-model-resolution.cpp Outdated

static const char * COMMIT = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";

struct http_client_stub : common_http_client {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

note that the same stub can be reused by other test cases in the future, we can make it more flexible if needed (TBD)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, moving it to a shared test header is easy once a second user shows up

@ServeurpersoCom

Copy link
Copy Markdown
Contributor Author

Remaining gpu-vulkan-intel-windows :

6
Start 26: test-model-resolution
25/33 Test #26: test-model-resolution .............***Exception: SegFault 0.24 sec

Comment thread tests/test-model-resolution.cpp Outdated
Comment on lines +461 to +464
common_http_client_set_factory([](const std::string & url) -> common_http_client_ptr {
return std::make_unique<http_client_stub>(url);
});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

not 100% sure about this, but probably you need to define the factory as a static func, not a lambda?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

A captureless lambda converts to a plain function pointer so both should be equivalent. But the latest commit makes the question moot for this test: the repos are now served by a real httplib server on the loopback and the test does not touch the factory anymore, so no C++ object crosses the module boundary at all, which is my best suspect for the Debug shared crash on the intel windows runner. The seam stays in common for future tests.

I will try replacing the stub by a mini httplib server on the loopback instead: for about the same line count the test becomes even more complete, the library exercises its real client and transport end to end, and no C++ object crosses the module boundary anymore, which is my best suspect for the Debug shared crash on the intel windows runner.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I see, yeah that makes sense. If the stub is defined at libcommon level, that would also solve the problem. but I'd agree that a loopback server works better in this case

Comment thread tests/test-model-resolution.cpp Outdated
Comment on lines +455 to +459
set_env("LLAMA_CACHE", cache_dir.string().c_str());

// an http endpoint keeps the client init from rejecting https on the
// builds without TLS support, the stub serves it either way
set_env("MODEL_ENDPOINT", "http://models.test/");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm, maybe also abstract out the set/get_env into common_get/set_env, so that it can be platform-independent similar to process.env in nodejs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea, and it composes with the latest commit: the stub is gone (the repos are now served by a real httplib server on the loopback so nothing crosses the module boundary anymore), but the test still sets LLAMA_CACHE and MODEL_ENDPOINT, so I will promote the helper into common_get/set_env and use it there.

@ServeurpersoCom

Copy link
Copy Markdown
Contributor Author

And now I can remove my http.cpp !

@ServeurpersoCom
ServeurpersoCom force-pushed the test-model-resolution branch from a915b7f to f6ac3ae Compare July 29, 2026 16:11
@ServeurpersoCom

Copy link
Copy Markdown
Contributor Author

ServeurpersoCom and others added 7 commits August 2, 2026 19:27
Include download.cpp and arg.cpp inside a namespace with hf_cache
monkey patched to serve hardcoded listings, so the resolution and the
model handler assembly are tested end-to-end through the real CLI
parsing, without modifying the tested code and without network access.

Covers the primary, shard, mmproj, sidecar and preset resolution on
layouts mimicking real vendor conventions, replays every case on
permutations of the listing to assert determinism, and asserts the
final wired paths for the spec type auto-selection, the -md precedence
and the fallback suppression.
Replace assert with a REQUIRE macro alive in Release builds, key the
fake hf_cache by repo id so the real handler init resolves every plan
itself, check the exact shard sets, restrict the permutation exception
to the order dependent picks, and cover dflash and eagle3.
The namespaced copy of the sources leaves many static functions unused
in this TU, exempt it from the unused warnings. Pre-include the
windows headers so arg.cpp does not pull them inside the namespace.
Declare the renamed copies of the download.h functions, verbatim from
the header and renamed in sync by the macros, so missing declaration
and missing prototype warnings are satisfied on every toolchain.
WIN32_LEAN_AND_MEAN and winsock2.h before windows.h, so http.h does
not redefine the socket types afterwards.
The test compiles its own copy of download.cpp, which calls httplib
directly, and the private link of llama-common does not propagate the
symbols under lld-link.
Add the virtual Head, Get and Post methods and the passthrough
setters to the common_http_client skeleton, move follow_location
into the constructor, expose the underlying client for the ranged
pull path, and rename the missed common_http_client_init call sites.
Replace the namespace inclusion of the sources by a plain TU: the
common_http_client factory returns a stub serving hardcoded HF API
responses, so the real hf_cache parsing, resolution and CLI handler
run against synthetic listings in an isolated cache directory.

Failures print the named case, the reordering and the actual versus
expected values, the assembly cases use the full command line as
context, and the empty result cases are checked once to keep the
logs short.
Assert the exact expected paths composed like the cache does instead of
suffix matching on forward slashes, set the environment portably, and
serve the stub through an http endpoint so the builds rejecting the
https scheme still reach it. Pause the log so the negative cases can be
replayed on every reordering.
Resume the paused log before the failure report so the CI shows why
the tested code bailed, and format the stub oids portably.
The factory was an inline variable, and the Windows shared builds
export functions but not data, so the executable and the DLL each had
their own instance: the stub installed by the test was invisible to
the library, which reached for the real endpoint and resolved nothing.
Route the creation through functions compiled into the library and
format the stub oids portably.
Replace the client stub by a real httplib server bound to the
loopback, so no C++ object crosses the module boundary anymore and
the library exercises its own client and transport end to end. The
debug shared build on Windows crashed inside the stubbed path.
The loopback server made the stub substitution unnecessary, the client
init builds the real client directly again.
The POSIX branch now behaves like _putenv_s, so the helper has a single
contract on every platform, and common_get_env already reads an unset and
an empty variable alike.

The model resolution test keys its cache directory on the loopback port,
where two concurrent runs on the same machine used to share one directory
and the initial cleanup of either wiped the other.
Comment thread common/http.h Outdated
}

static std::pair<httplib::Client, common_http_url> common_http_client(const std::string & url) {
class common_http_client {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this wrapper is unused now, maybe better to remove it?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

(I mean revert to the non-ptr version on master)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@ngxson ngxson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nice, very clean now

@ServeurpersoCom
ServeurpersoCom merged commit ee0445c into ggml-org:master Aug 3, 2026
22 of 26 checks passed
smalinin pushed a commit to smalinin/llama.cpp that referenced this pull request Aug 4, 2026
…#26172)

* tests: add model resolution test on synthetic repo listings

Include download.cpp and arg.cpp inside a namespace with hf_cache
monkey patched to serve hardcoded listings, so the resolution and the
model handler assembly are tested end-to-end through the real CLI
parsing, without modifying the tested code and without network access.

Covers the primary, shard, mmproj, sidecar and preset resolution on
layouts mimicking real vendor conventions, replays every case on
permutations of the listing to assert determinism, and asserts the
final wired paths for the spec type auto-selection, the -md precedence
and the fallback suppression.

* tests: keep model resolution checks active and let the handler resolve

Replace assert with a REQUIRE macro alive in Release builds, key the
fake hf_cache by repo id so the real handler init resolves every plan
itself, check the exact shard sets, restrict the permutation exception
to the order dependent picks, and cover dflash and eagle3.

* tests: fix model resolution build on fatal warnings CI and Windows

The namespaced copy of the sources leaves many static functions unused
in this TU, exempt it from the unused warnings. Pre-include the
windows headers so arg.cpp does not pull them inside the namespace.
Declare the renamed copies of the download.h functions, verbatim from
the header and renamed in sync by the macros, so missing declaration
and missing prototype warnings are satisfied on every toolchain.

* tests: fix winsock inclusion order for the model resolution test

WIN32_LEAN_AND_MEAN and winsock2.h before windows.h, so http.h does
not redefine the socket types afterwards.

* tests: link cpp-httplib to the model resolution test

The test compiles its own copy of download.cpp, which calls httplib
directly, and the private link of llama-common does not propagate the
symbols under lld-link.

* common_http_client

* common: finish the http client wrapper

Add the virtual Head, Get and Post methods and the passthrough
setters to the common_http_client skeleton, move follow_location
into the constructor, expose the underlying client for the ranged
pull path, and rename the missed common_http_client_init call sites.

* tests: rewrite model resolution on the http client stub

Replace the namespace inclusion of the sources by a plain TU: the
common_http_client factory returns a stub serving hardcoded HF API
responses, so the real hf_cache parsing, resolution and CLI handler
run against synthetic listings in an isolated cache directory.

Failures print the named case, the reordering and the actual versus
expected values, the assembly cases use the full command line as
context, and the empty result cases are checked once to keep the
logs short.

* tests: fix the model resolution on Windows and the builds without TLS

Assert the exact expected paths composed like the cache does instead of
suffix matching on forward slashes, set the environment portably, and
serve the stub through an http endpoint so the builds rejecting the
https scheme still reach it. Pause the log so the negative cases can be
replayed on every reordering.

* tests: make the model resolution failures self explanatory

Resume the paused log before the failure report so the CI shows why
the tested code bailed, and format the stub oids portably.

* common: hold the http client factory behind exported functions

The factory was an inline variable, and the Windows shared builds
export functions but not data, so the executable and the DLL each had
their own instance: the stub installed by the test was invisible to
the library, which reached for the real endpoint and resolved nothing.
Route the creation through functions compiled into the library and
format the stub oids portably.

* common: add the http client factory source missed in the previous commit

* common: typedef the http client factory callback

Address review from @ngxson

* tests: serve the model resolution repos over the loopback

Replace the client stub by a real httplib server bound to the
loopback, so no C++ object crosses the module boundary anymore and
the library exercises its own client and transport end to end. The
debug shared build on Windows crashed inside the stubbed path.

* common: add portable common_get_env and common_set_env helpers

Address review from @ngxson

* common: drop the http client factory left without a caller

The loopback server made the stub substitution unnecessary, the client
init builds the real client directly again.

* common: read the model endpoint through the env helpers

* nit: drop the stub leftovers from the model resolution test

* common: align common_set_env and isolate the test cache per run

The POSIX branch now behaves like _putenv_s, so the helper has a single
contract on every platform, and common_get_env already reads an unset and
an empty variable alike.

The model resolution test keys its cache directory on the loopback port,
where two concurrent runs on the same machine used to share one directory
and the initial cleanup of either wiped the other.

* tests: move the model resolution server into main

* tests: support the DSpark sidecar resolution

* common: revert the http client to the plain httplib client

address review from @ngxson

---------

Co-authored-by: Xuan Son Nguyen <son@huggingface.co>
satindergrewal pushed a commit to satindergrewal/llama.cpp that referenced this pull request Aug 11, 2026
…#26172)

* tests: add model resolution test on synthetic repo listings

Include download.cpp and arg.cpp inside a namespace with hf_cache
monkey patched to serve hardcoded listings, so the resolution and the
model handler assembly are tested end-to-end through the real CLI
parsing, without modifying the tested code and without network access.

Covers the primary, shard, mmproj, sidecar and preset resolution on
layouts mimicking real vendor conventions, replays every case on
permutations of the listing to assert determinism, and asserts the
final wired paths for the spec type auto-selection, the -md precedence
and the fallback suppression.

* tests: keep model resolution checks active and let the handler resolve

Replace assert with a REQUIRE macro alive in Release builds, key the
fake hf_cache by repo id so the real handler init resolves every plan
itself, check the exact shard sets, restrict the permutation exception
to the order dependent picks, and cover dflash and eagle3.

* tests: fix model resolution build on fatal warnings CI and Windows

The namespaced copy of the sources leaves many static functions unused
in this TU, exempt it from the unused warnings. Pre-include the
windows headers so arg.cpp does not pull them inside the namespace.
Declare the renamed copies of the download.h functions, verbatim from
the header and renamed in sync by the macros, so missing declaration
and missing prototype warnings are satisfied on every toolchain.

* tests: fix winsock inclusion order for the model resolution test

WIN32_LEAN_AND_MEAN and winsock2.h before windows.h, so http.h does
not redefine the socket types afterwards.

* tests: link cpp-httplib to the model resolution test

The test compiles its own copy of download.cpp, which calls httplib
directly, and the private link of llama-common does not propagate the
symbols under lld-link.

* common_http_client

* common: finish the http client wrapper

Add the virtual Head, Get and Post methods and the passthrough
setters to the common_http_client skeleton, move follow_location
into the constructor, expose the underlying client for the ranged
pull path, and rename the missed common_http_client_init call sites.

* tests: rewrite model resolution on the http client stub

Replace the namespace inclusion of the sources by a plain TU: the
common_http_client factory returns a stub serving hardcoded HF API
responses, so the real hf_cache parsing, resolution and CLI handler
run against synthetic listings in an isolated cache directory.

Failures print the named case, the reordering and the actual versus
expected values, the assembly cases use the full command line as
context, and the empty result cases are checked once to keep the
logs short.

* tests: fix the model resolution on Windows and the builds without TLS

Assert the exact expected paths composed like the cache does instead of
suffix matching on forward slashes, set the environment portably, and
serve the stub through an http endpoint so the builds rejecting the
https scheme still reach it. Pause the log so the negative cases can be
replayed on every reordering.

* tests: make the model resolution failures self explanatory

Resume the paused log before the failure report so the CI shows why
the tested code bailed, and format the stub oids portably.

* common: hold the http client factory behind exported functions

The factory was an inline variable, and the Windows shared builds
export functions but not data, so the executable and the DLL each had
their own instance: the stub installed by the test was invisible to
the library, which reached for the real endpoint and resolved nothing.
Route the creation through functions compiled into the library and
format the stub oids portably.

* common: add the http client factory source missed in the previous commit

* common: typedef the http client factory callback

Address review from @ngxson

* tests: serve the model resolution repos over the loopback

Replace the client stub by a real httplib server bound to the
loopback, so no C++ object crosses the module boundary anymore and
the library exercises its own client and transport end to end. The
debug shared build on Windows crashed inside the stubbed path.

* common: add portable common_get_env and common_set_env helpers

Address review from @ngxson

* common: drop the http client factory left without a caller

The loopback server made the stub substitution unnecessary, the client
init builds the real client directly again.

* common: read the model endpoint through the env helpers

* nit: drop the stub leftovers from the model resolution test

* common: align common_set_env and isolate the test cache per run

The POSIX branch now behaves like _putenv_s, so the helper has a single
contract on every platform, and common_get_env already reads an unset and
an empty variable alike.

The model resolution test keys its cache directory on the loopback port,
where two concurrent runs on the same machine used to share one directory
and the initial cleanup of either wiped the other.

* tests: move the model resolution server into main

* tests: support the DSpark sidecar resolution

* common: revert the http client to the plain httplib client

address review from @ngxson

---------

Co-authored-by: Xuan Son Nguyen <son@huggingface.co>
satindergrewal pushed a commit to satindergrewal/llama.cpp that referenced this pull request Aug 12, 2026
…#26172)

* tests: add model resolution test on synthetic repo listings

Include download.cpp and arg.cpp inside a namespace with hf_cache
monkey patched to serve hardcoded listings, so the resolution and the
model handler assembly are tested end-to-end through the real CLI
parsing, without modifying the tested code and without network access.

Covers the primary, shard, mmproj, sidecar and preset resolution on
layouts mimicking real vendor conventions, replays every case on
permutations of the listing to assert determinism, and asserts the
final wired paths for the spec type auto-selection, the -md precedence
and the fallback suppression.

* tests: keep model resolution checks active and let the handler resolve

Replace assert with a REQUIRE macro alive in Release builds, key the
fake hf_cache by repo id so the real handler init resolves every plan
itself, check the exact shard sets, restrict the permutation exception
to the order dependent picks, and cover dflash and eagle3.

* tests: fix model resolution build on fatal warnings CI and Windows

The namespaced copy of the sources leaves many static functions unused
in this TU, exempt it from the unused warnings. Pre-include the
windows headers so arg.cpp does not pull them inside the namespace.
Declare the renamed copies of the download.h functions, verbatim from
the header and renamed in sync by the macros, so missing declaration
and missing prototype warnings are satisfied on every toolchain.

* tests: fix winsock inclusion order for the model resolution test

WIN32_LEAN_AND_MEAN and winsock2.h before windows.h, so http.h does
not redefine the socket types afterwards.

* tests: link cpp-httplib to the model resolution test

The test compiles its own copy of download.cpp, which calls httplib
directly, and the private link of llama-common does not propagate the
symbols under lld-link.

* common_http_client

* common: finish the http client wrapper

Add the virtual Head, Get and Post methods and the passthrough
setters to the common_http_client skeleton, move follow_location
into the constructor, expose the underlying client for the ranged
pull path, and rename the missed common_http_client_init call sites.

* tests: rewrite model resolution on the http client stub

Replace the namespace inclusion of the sources by a plain TU: the
common_http_client factory returns a stub serving hardcoded HF API
responses, so the real hf_cache parsing, resolution and CLI handler
run against synthetic listings in an isolated cache directory.

Failures print the named case, the reordering and the actual versus
expected values, the assembly cases use the full command line as
context, and the empty result cases are checked once to keep the
logs short.

* tests: fix the model resolution on Windows and the builds without TLS

Assert the exact expected paths composed like the cache does instead of
suffix matching on forward slashes, set the environment portably, and
serve the stub through an http endpoint so the builds rejecting the
https scheme still reach it. Pause the log so the negative cases can be
replayed on every reordering.

* tests: make the model resolution failures self explanatory

Resume the paused log before the failure report so the CI shows why
the tested code bailed, and format the stub oids portably.

* common: hold the http client factory behind exported functions

The factory was an inline variable, and the Windows shared builds
export functions but not data, so the executable and the DLL each had
their own instance: the stub installed by the test was invisible to
the library, which reached for the real endpoint and resolved nothing.
Route the creation through functions compiled into the library and
format the stub oids portably.

* common: add the http client factory source missed in the previous commit

* common: typedef the http client factory callback

Address review from @ngxson

* tests: serve the model resolution repos over the loopback

Replace the client stub by a real httplib server bound to the
loopback, so no C++ object crosses the module boundary anymore and
the library exercises its own client and transport end to end. The
debug shared build on Windows crashed inside the stubbed path.

* common: add portable common_get_env and common_set_env helpers

Address review from @ngxson

* common: drop the http client factory left without a caller

The loopback server made the stub substitution unnecessary, the client
init builds the real client directly again.

* common: read the model endpoint through the env helpers

* nit: drop the stub leftovers from the model resolution test

* common: align common_set_env and isolate the test cache per run

The POSIX branch now behaves like _putenv_s, so the helper has a single
contract on every platform, and common_get_env already reads an unset and
an empty variable alike.

The model resolution test keys its cache directory on the loopback port,
where two concurrent runs on the same machine used to share one directory
and the initial cleanup of either wiped the other.

* tests: move the model resolution server into main

* tests: support the DSpark sidecar resolution

* common: revert the http client to the plain httplib client

address review from @ngxson

---------

Co-authored-by: Xuan Son Nguyen <son@huggingface.co>
CowboyTim pushed a commit to aardbeiplantje/llama.cpp that referenced this pull request Aug 13, 2026
…#26172)

* tests: add model resolution test on synthetic repo listings

Include download.cpp and arg.cpp inside a namespace with hf_cache
monkey patched to serve hardcoded listings, so the resolution and the
model handler assembly are tested end-to-end through the real CLI
parsing, without modifying the tested code and without network access.

Covers the primary, shard, mmproj, sidecar and preset resolution on
layouts mimicking real vendor conventions, replays every case on
permutations of the listing to assert determinism, and asserts the
final wired paths for the spec type auto-selection, the -md precedence
and the fallback suppression.

* tests: keep model resolution checks active and let the handler resolve

Replace assert with a REQUIRE macro alive in Release builds, key the
fake hf_cache by repo id so the real handler init resolves every plan
itself, check the exact shard sets, restrict the permutation exception
to the order dependent picks, and cover dflash and eagle3.

* tests: fix model resolution build on fatal warnings CI and Windows

The namespaced copy of the sources leaves many static functions unused
in this TU, exempt it from the unused warnings. Pre-include the
windows headers so arg.cpp does not pull them inside the namespace.
Declare the renamed copies of the download.h functions, verbatim from
the header and renamed in sync by the macros, so missing declaration
and missing prototype warnings are satisfied on every toolchain.

* tests: fix winsock inclusion order for the model resolution test

WIN32_LEAN_AND_MEAN and winsock2.h before windows.h, so http.h does
not redefine the socket types afterwards.

* tests: link cpp-httplib to the model resolution test

The test compiles its own copy of download.cpp, which calls httplib
directly, and the private link of llama-common does not propagate the
symbols under lld-link.

* common_http_client

* common: finish the http client wrapper

Add the virtual Head, Get and Post methods and the passthrough
setters to the common_http_client skeleton, move follow_location
into the constructor, expose the underlying client for the ranged
pull path, and rename the missed common_http_client_init call sites.

* tests: rewrite model resolution on the http client stub

Replace the namespace inclusion of the sources by a plain TU: the
common_http_client factory returns a stub serving hardcoded HF API
responses, so the real hf_cache parsing, resolution and CLI handler
run against synthetic listings in an isolated cache directory.

Failures print the named case, the reordering and the actual versus
expected values, the assembly cases use the full command line as
context, and the empty result cases are checked once to keep the
logs short.

* tests: fix the model resolution on Windows and the builds without TLS

Assert the exact expected paths composed like the cache does instead of
suffix matching on forward slashes, set the environment portably, and
serve the stub through an http endpoint so the builds rejecting the
https scheme still reach it. Pause the log so the negative cases can be
replayed on every reordering.

* tests: make the model resolution failures self explanatory

Resume the paused log before the failure report so the CI shows why
the tested code bailed, and format the stub oids portably.

* common: hold the http client factory behind exported functions

The factory was an inline variable, and the Windows shared builds
export functions but not data, so the executable and the DLL each had
their own instance: the stub installed by the test was invisible to
the library, which reached for the real endpoint and resolved nothing.
Route the creation through functions compiled into the library and
format the stub oids portably.

* common: add the http client factory source missed in the previous commit

* common: typedef the http client factory callback

Address review from @ngxson

* tests: serve the model resolution repos over the loopback

Replace the client stub by a real httplib server bound to the
loopback, so no C++ object crosses the module boundary anymore and
the library exercises its own client and transport end to end. The
debug shared build on Windows crashed inside the stubbed path.

* common: add portable common_get_env and common_set_env helpers

Address review from @ngxson

* common: drop the http client factory left without a caller

The loopback server made the stub substitution unnecessary, the client
init builds the real client directly again.

* common: read the model endpoint through the env helpers

* nit: drop the stub leftovers from the model resolution test

* common: align common_set_env and isolate the test cache per run

The POSIX branch now behaves like _putenv_s, so the helper has a single
contract on every platform, and common_get_env already reads an unset and
an empty variable alike.

The model resolution test keys its cache directory on the loopback port,
where two concurrent runs on the same machine used to share one directory
and the initial cleanup of either wiped the other.

* tests: move the model resolution server into main

* tests: support the DSpark sidecar resolution

* common: revert the http client to the plain httplib client

address review from @ngxson

---------

Co-authored-by: Xuan Son Nguyen <son@huggingface.co>
brittlewis12 pushed a commit to brittlewis12/llama.cpp that referenced this pull request Aug 17, 2026
…#26172)

* tests: add model resolution test on synthetic repo listings

Include download.cpp and arg.cpp inside a namespace with hf_cache
monkey patched to serve hardcoded listings, so the resolution and the
model handler assembly are tested end-to-end through the real CLI
parsing, without modifying the tested code and without network access.

Covers the primary, shard, mmproj, sidecar and preset resolution on
layouts mimicking real vendor conventions, replays every case on
permutations of the listing to assert determinism, and asserts the
final wired paths for the spec type auto-selection, the -md precedence
and the fallback suppression.

* tests: keep model resolution checks active and let the handler resolve

Replace assert with a REQUIRE macro alive in Release builds, key the
fake hf_cache by repo id so the real handler init resolves every plan
itself, check the exact shard sets, restrict the permutation exception
to the order dependent picks, and cover dflash and eagle3.

* tests: fix model resolution build on fatal warnings CI and Windows

The namespaced copy of the sources leaves many static functions unused
in this TU, exempt it from the unused warnings. Pre-include the
windows headers so arg.cpp does not pull them inside the namespace.
Declare the renamed copies of the download.h functions, verbatim from
the header and renamed in sync by the macros, so missing declaration
and missing prototype warnings are satisfied on every toolchain.

* tests: fix winsock inclusion order for the model resolution test

WIN32_LEAN_AND_MEAN and winsock2.h before windows.h, so http.h does
not redefine the socket types afterwards.

* tests: link cpp-httplib to the model resolution test

The test compiles its own copy of download.cpp, which calls httplib
directly, and the private link of llama-common does not propagate the
symbols under lld-link.

* common_http_client

* common: finish the http client wrapper

Add the virtual Head, Get and Post methods and the passthrough
setters to the common_http_client skeleton, move follow_location
into the constructor, expose the underlying client for the ranged
pull path, and rename the missed common_http_client_init call sites.

* tests: rewrite model resolution on the http client stub

Replace the namespace inclusion of the sources by a plain TU: the
common_http_client factory returns a stub serving hardcoded HF API
responses, so the real hf_cache parsing, resolution and CLI handler
run against synthetic listings in an isolated cache directory.

Failures print the named case, the reordering and the actual versus
expected values, the assembly cases use the full command line as
context, and the empty result cases are checked once to keep the
logs short.

* tests: fix the model resolution on Windows and the builds without TLS

Assert the exact expected paths composed like the cache does instead of
suffix matching on forward slashes, set the environment portably, and
serve the stub through an http endpoint so the builds rejecting the
https scheme still reach it. Pause the log so the negative cases can be
replayed on every reordering.

* tests: make the model resolution failures self explanatory

Resume the paused log before the failure report so the CI shows why
the tested code bailed, and format the stub oids portably.

* common: hold the http client factory behind exported functions

The factory was an inline variable, and the Windows shared builds
export functions but not data, so the executable and the DLL each had
their own instance: the stub installed by the test was invisible to
the library, which reached for the real endpoint and resolved nothing.
Route the creation through functions compiled into the library and
format the stub oids portably.

* common: add the http client factory source missed in the previous commit

* common: typedef the http client factory callback

Address review from @ngxson

* tests: serve the model resolution repos over the loopback

Replace the client stub by a real httplib server bound to the
loopback, so no C++ object crosses the module boundary anymore and
the library exercises its own client and transport end to end. The
debug shared build on Windows crashed inside the stubbed path.

* common: add portable common_get_env and common_set_env helpers

Address review from @ngxson

* common: drop the http client factory left without a caller

The loopback server made the stub substitution unnecessary, the client
init builds the real client directly again.

* common: read the model endpoint through the env helpers

* nit: drop the stub leftovers from the model resolution test

* common: align common_set_env and isolate the test cache per run

The POSIX branch now behaves like _putenv_s, so the helper has a single
contract on every platform, and common_get_env already reads an unset and
an empty variable alike.

The model resolution test keys its cache directory on the loopback port,
where two concurrent runs on the same machine used to share one directory
and the initial cleanup of either wiped the other.

* tests: move the model resolution server into main

* tests: support the DSpark sidecar resolution

* common: revert the http client to the plain httplib client

address review from @ngxson

---------

Co-authored-by: Xuan Son Nguyen <son@huggingface.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

examples testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants