Skip to content

fix: clean up test imports#1600

Merged
yanxi0830 merged 16 commits intomainfrom
delete_unused_imports
Mar 13, 2025
Merged

fix: clean up test imports#1600
yanxi0830 merged 16 commits intomainfrom
delete_unused_imports

Conversation

@yanxi0830
Copy link
Copy Markdown
Contributor

@yanxi0830 yanxi0830 commented Mar 12, 2025

What does this PR do?

Test Plan

pytest -v -s --nbval-lax ./docs/getting_started.ipynb

LLAMA_STACK_CONFIG=fireworks pytest -v tests/integration/ --text-model meta-llama/Llama-3.3-70B-Instruct
image

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Mar 12, 2025
Copy link
Copy Markdown
Contributor

@booxter booxter left a comment

Choose a reason for hiding this comment

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

Thanks!



@pytest.fixture
def test_dataset(llama_stack_client):
Copy link
Copy Markdown
Contributor

@ashwinb ashwinb Mar 13, 2025

Choose a reason for hiding this comment

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

  • can we call this something not with a test_ prefix because that is how tests are typically marked. maybe dataset_for_test?
  • also we call unregister() manually with the "test_dataset" argument but register_dataset() is not passed that argument?

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.

renamed fixture & register dataset with explicit id

@yanxi0830 yanxi0830 merged commit 98811cc into main Mar 13, 2025
7 checks passed
@yanxi0830 yanxi0830 deleted the delete_unused_imports branch March 13, 2025 18:01
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.

4 participants