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

Do not require all dummy api keys in inference tests #1226

Merged
merged 3 commits into from
Mar 4, 2025

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Feb 25, 2025

I currently find it annoying when I want to run tests locally (that already have cached tapes) to set the env variables to "dummy":

HF_FAL_KEY=dummy  pnpm run test -t "Fal AI"

This PR is a suggestion to get rid of that and default to "dummy" in the code instead of in the CI workflow file. Happy to get better suggestions if you think of one (in Python I would do an automatic fixture "get_api_key_or_dummy" but quite specific to pytest).

As a bonus, no need to update .github/workflows/test.yml when adding a new provider.

WDYT?

@@ -778,7 +778,7 @@ describe.concurrent("HfInference", () => {
describe.concurrent(
"Fal AI",
() => {
const client = new HfInference(env.HF_FAL_KEY);
Copy link
Member

Choose a reason for hiding this comment

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

use ?? instead of || (same for others)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved in 664a38a

@Wauplin Wauplin merged commit 6e65421 into main Mar 4, 2025
5 checks passed
@Wauplin Wauplin deleted the do-not-define-dummy-variables-in-ci branch March 4, 2025 11:03
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.

2 participants