mirror of
https://github.com/meta-llama/llama-stack.git
synced 2025-07-30 07:39:38 +00:00
fix: Fix unit tests CI and failing tests (#2928)
# What does this PR do? <!-- Provide a short summary of what this PR does and why. Link to relevant issues if applicable. --> - Added `set -e` to the beginning of the unit test script to ensure the script exits on failure and correctly fails the CI when tests do not pass. - Fixed all unit tests that were silently failing in the CI. - Fixed Python 3.13 unit test CI failing silently. <!-- If resolving an issue, uncomment and update the line below --> <!-- Closes #[issue-number] --> Closes #2877 ## Test Plan <!-- Describe the tests you ran to verify your changes with result summaries. *Provide clear instructions so the plan can be easily re-executed.* --> - **Previously:** Unit tests passing in CI eventhough it failed 11 tests -> [CI-run](4683681501 (step)
:4:2097) - **Made the fix. Now, ensuring CI fails as expected on test failures:** Unit tests failing in CI with 1 failed test -> [CI-run](4684234247 (step)
:4:1506) - This PR shows the CI passing and all unit tests passing.
This commit is contained in:
parent
46e2989312
commit
c48dcafc77
5 changed files with 30 additions and 21 deletions
2
.github/workflows/unit-tests.yml
vendored
2
.github/workflows/unit-tests.yml
vendored
|
@ -35,6 +35,8 @@ jobs:
|
||||||
|
|
||||||
- name: Install dependencies
|
- name: Install dependencies
|
||||||
uses: ./.github/actions/setup-runner
|
uses: ./.github/actions/setup-runner
|
||||||
|
with:
|
||||||
|
python-version: ${{ matrix.python }}
|
||||||
|
|
||||||
- name: Run unit tests
|
- name: Run unit tests
|
||||||
run: |
|
run: |
|
||||||
|
|
|
@ -8,6 +8,15 @@
|
||||||
|
|
||||||
PYTHON_VERSION=${PYTHON_VERSION:-3.12}
|
PYTHON_VERSION=${PYTHON_VERSION:-3.12}
|
||||||
|
|
||||||
|
set -e
|
||||||
|
|
||||||
|
# Always run this at the end, even if something fails
|
||||||
|
cleanup() {
|
||||||
|
echo "Generating coverage report..."
|
||||||
|
uv run --python "$PYTHON_VERSION" coverage html -d htmlcov-$PYTHON_VERSION
|
||||||
|
}
|
||||||
|
trap cleanup EXIT
|
||||||
|
|
||||||
command -v uv >/dev/null 2>&1 || { echo >&2 "uv is required but it's not installed. Exiting."; exit 1; }
|
command -v uv >/dev/null 2>&1 || { echo >&2 "uv is required but it's not installed. Exiting."; exit 1; }
|
||||||
|
|
||||||
uv python find "$PYTHON_VERSION"
|
uv python find "$PYTHON_VERSION"
|
||||||
|
@ -19,6 +28,3 @@ fi
|
||||||
# Run unit tests with coverage
|
# Run unit tests with coverage
|
||||||
uv run --python "$PYTHON_VERSION" --with-editable . --group unit \
|
uv run --python "$PYTHON_VERSION" --with-editable . --group unit \
|
||||||
coverage run --source=llama_stack -m pytest -s -v tests/unit/ "$@"
|
coverage run --source=llama_stack -m pytest -s -v tests/unit/ "$@"
|
||||||
|
|
||||||
# Generate HTML coverage report
|
|
||||||
uv run --python "$PYTHON_VERSION" coverage html -d htmlcov-$PYTHON_VERSION
|
|
||||||
|
|
|
@ -346,7 +346,7 @@ pip_packages:
|
||||||
|
|
||||||
def test_external_provider_from_module_building(self, mock_providers):
|
def test_external_provider_from_module_building(self, mock_providers):
|
||||||
"""Test loading an external provider from a module during build (building=True, partial spec)."""
|
"""Test loading an external provider from a module during build (building=True, partial spec)."""
|
||||||
from llama_stack.distribution.datatypes import BuildConfig, DistributionSpec, Provider
|
from llama_stack.distribution.datatypes import BuildConfig, BuildProvider, DistributionSpec
|
||||||
from llama_stack.providers.datatypes import Api
|
from llama_stack.providers.datatypes import Api
|
||||||
|
|
||||||
# No importlib patch needed, should not import module when type of `config` is BuildConfig or DistributionSpec
|
# No importlib patch needed, should not import module when type of `config` is BuildConfig or DistributionSpec
|
||||||
|
@ -358,10 +358,8 @@ pip_packages:
|
||||||
description="test",
|
description="test",
|
||||||
providers={
|
providers={
|
||||||
"inference": [
|
"inference": [
|
||||||
Provider(
|
BuildProvider(
|
||||||
provider_id="external_test",
|
|
||||||
provider_type="external_test",
|
provider_type="external_test",
|
||||||
config={},
|
|
||||||
module="external_test",
|
module="external_test",
|
||||||
)
|
)
|
||||||
]
|
]
|
||||||
|
|
|
@ -162,26 +162,29 @@ async def test_register_model_existing_different(
|
||||||
await helper.register_model(known_model)
|
await helper.register_model(known_model)
|
||||||
|
|
||||||
|
|
||||||
async def test_unregister_model(helper: ModelRegistryHelper, known_model: Model) -> None:
|
# TODO: unregister_model functionality was removed/disabled by https://github.com/meta-llama/llama-stack/pull/2916
|
||||||
await helper.register_model(known_model) # duplicate entry
|
# async def test_unregister_model(helper: ModelRegistryHelper, known_model: Model) -> None:
|
||||||
assert helper.get_provider_model_id(known_model.model_id) == known_model.provider_model_id
|
# await helper.register_model(known_model) # duplicate entry
|
||||||
await helper.unregister_model(known_model.model_id)
|
# assert helper.get_provider_model_id(known_model.model_id) == known_model.provider_model_id
|
||||||
assert helper.get_provider_model_id(known_model.model_id) is None
|
# await helper.unregister_model(known_model.model_id)
|
||||||
|
# assert helper.get_provider_model_id(known_model.model_id) is None
|
||||||
|
|
||||||
|
|
||||||
async def test_unregister_unknown_model(helper: ModelRegistryHelper, unknown_model: Model) -> None:
|
# TODO: unregister_model functionality was removed/disabled by https://github.com/meta-llama/llama-stack/pull/2916
|
||||||
with pytest.raises(ValueError):
|
# async def test_unregister_unknown_model(helper: ModelRegistryHelper, unknown_model: Model) -> None:
|
||||||
await helper.unregister_model(unknown_model.model_id)
|
# with pytest.raises(ValueError):
|
||||||
|
# await helper.unregister_model(unknown_model.model_id)
|
||||||
|
|
||||||
|
|
||||||
async def test_register_model_during_init(helper: ModelRegistryHelper, known_model: Model) -> None:
|
async def test_register_model_during_init(helper: ModelRegistryHelper, known_model: Model) -> None:
|
||||||
assert helper.get_provider_model_id(known_model.provider_resource_id) == known_model.provider_model_id
|
assert helper.get_provider_model_id(known_model.provider_resource_id) == known_model.provider_model_id
|
||||||
|
|
||||||
|
|
||||||
async def test_unregister_model_during_init(helper: ModelRegistryHelper, known_model: Model) -> None:
|
# TODO: unregister_model functionality was removed/disabled by https://github.com/meta-llama/llama-stack/pull/2916
|
||||||
assert helper.get_provider_model_id(known_model.provider_resource_id) == known_model.provider_model_id
|
# async def test_unregister_model_during_init(helper: ModelRegistryHelper, known_model: Model) -> None:
|
||||||
await helper.unregister_model(known_model.provider_resource_id)
|
# assert helper.get_provider_model_id(known_model.provider_resource_id) == known_model.provider_model_id
|
||||||
assert helper.get_provider_model_id(known_model.provider_resource_id) is None
|
# await helper.unregister_model(known_model.provider_resource_id)
|
||||||
|
# assert helper.get_provider_model_id(known_model.provider_resource_id) is None
|
||||||
|
|
||||||
|
|
||||||
async def test_register_model_from_check_model_availability(
|
async def test_register_model_from_check_model_availability(
|
||||||
|
|
|
@ -49,7 +49,7 @@ def github_token_app():
|
||||||
)
|
)
|
||||||
|
|
||||||
# Add auth middleware
|
# Add auth middleware
|
||||||
app.add_middleware(AuthenticationMiddleware, auth_config=auth_config)
|
app.add_middleware(AuthenticationMiddleware, auth_config=auth_config, impls={})
|
||||||
|
|
||||||
@app.get("/test")
|
@app.get("/test")
|
||||||
def test_endpoint():
|
def test_endpoint():
|
||||||
|
@ -149,7 +149,7 @@ def test_github_enterprise_support(mock_client_class):
|
||||||
access_policy=[],
|
access_policy=[],
|
||||||
)
|
)
|
||||||
|
|
||||||
app.add_middleware(AuthenticationMiddleware, auth_config=auth_config)
|
app.add_middleware(AuthenticationMiddleware, auth_config=auth_config, impls={})
|
||||||
|
|
||||||
@app.get("/test")
|
@app.get("/test")
|
||||||
def test_endpoint():
|
def test_endpoint():
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue