Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add pytest option to generate a functional report for distribution #833

Merged
merged 4 commits into from
Jan 22, 2025

Conversation

sixianyi0721
Copy link
Contributor

@sixianyi0721 sixianyi0721 commented Jan 22, 2025

What does this PR do?

add pytest option (--report) to support generating a functional report for llama stack distribution

Test Plan

export LLAMA_STACK_CONFIG=./llama_stack/templates/fireworks/run.yaml
/opt/miniconda3/envs/stack/bin/pytest -s -v tests/client-sdk/  --report

See a report file was generated under ./llama_stack/templates/fireworks/report.md

Sources

Please link relevant resources if necessary.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Ran pre-commit to handle lint / formatting issues.
  • Read the contributor guideline,
    Pull Request section?
  • Updated relevant documentation.
  • Wrote necessary unit or integration tests.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jan 22, 2025
@sixianyi0721 sixianyi0721 force-pushed the client-sdk-test branch 2 times, most recently from d8ec744 to 0246333 Compare January 22, 2025 01:58
@sixianyi0721 sixianyi0721 marked this pull request as ready for review January 22, 2025 01:59
tests/client-sdk/report.py Outdated Show resolved Hide resolved
tests/client-sdk/report.py Outdated Show resolved Hide resolved
tests/client-sdk/report.py Show resolved Hide resolved
rows = []
for model in all_registered_models():
if (
"Instruct" not in model.core_model_id.value
Copy link
Contributor

Choose a reason for hiding this comment

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

model.is_instruct_model() might be better to use for instruct models.
and for safety models, might be better to check model family and if its ModelFamily.safety

INFERENCE_API_CAPA_TEST_MAP = {
"chat_completion": {
"streaming": [
"test_text_chat_completion_streaming",
Copy link
Contributor

Choose a reason for hiding this comment

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

this implies that we need to manually add tests to this list , everytime we add new capabilities. Not ideal but something we can look into as a follow up .

Copy link
Contributor Author

@sixianyi0721 sixianyi0721 Jan 22, 2025

Choose a reason for hiding this comment

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

@hardikjshah
Since we want to find out the corresponding tests given an api and the capability, the other approaches are

  1. parse test function name
  2. add markers (ex. pytest.marker.structured_output) to each test

I think both would be less cleaner and therefore i prefer the current approach. Lemme know if you have other suggestions! :)

tests/client-sdk/metadata.py Show resolved Hide resolved
@sixianyi0721 sixianyi0721 force-pushed the client-sdk-test branch 2 times, most recently from c9fc1c0 to 2ace8d2 Compare January 22, 2025 02:51
@sixianyi0721 sixianyi0721 merged commit edf5688 into main Jan 22, 2025
2 checks passed
@sixianyi0721 sixianyi0721 deleted the client-sdk-test branch January 22, 2025 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants