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

OPENAI_KEY as env variable and extra tests #27

Merged
merged 10 commits into from
Apr 29, 2024
Merged

Conversation

vpapanasta
Copy link
Member

In this change, I am setting secrets.OPENAI_KEY as an environment variable that we can use in tests.
Also, I am adding some tests to increase code coverage in openAIChat. One test of these uses the secret OPENAI_KEY, and it was added to check that we can use the key.

Checking secrets key configuration in yml config file
Add first test to check secret PENAI_KEY
Add extra test to increase code coverage in openAIChat
Adding negative tests to increase code coverage in openAIMessages/validateAssistantWithToolCalls
@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

Adding extra tests to increase code coverage in extractOpenAIEmbeddings
Add openAIImages positive test using OPENAI_KEY to increase code coverage
tests/topenAIChat.m Outdated Show resolved Hide resolved
Changing createOpenAIChatWithStreamFunc as suggested by Christopher
tests/topenAIChat.m Show resolved Hide resolved
@@ -13,6 +13,8 @@ jobs:
products: Text_Analytics_Toolbox
cache: true
- name: Run tests and generate artifacts
env:
OPENAI_KEY: ${{ secrets.OPENAI_KEY }}
Copy link
Member

Choose a reason for hiding this comment

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

OPENAI_API_KEY to match the env variable that we expect?

We might have to remove this part from the test:

function saveEnvVar(testCase)
           % Ensures key is not in environment variable for tests
           openAIEnvVar = "OPENAI_API_KEY";
           if isenv(openAIEnvVar)
               key = getenv(openAIEnvVar);
               unsetenv(openAIEnvVar);
               testCase.addTeardown(@(x) setenv(openAIEnvVar, x), key);
           end
       end

I have added that because when I was running local tests on some behaviors related to checking the key it would turn out that the key was already in my path. We should just move that to where it's actually needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

For local debugging we should still use the manually defined OPENAI_API_KEY. I do not think it harms to keep this check to make sure that OPENAI_API_KEY is not in environment variable for tests. OPENAI_KEY defined in .github/workflows/ci.yml is a different environment variable and it is secret.

@vpapanasta vpapanasta merged commit aabf577 into main Apr 29, 2024
1 check passed
@vpapanasta vpapanasta deleted the tests_using_key branch April 29, 2024 11:04
@vpapanasta vpapanasta restored the tests_using_key branch April 30, 2024 15:10
@vpapanasta vpapanasta deleted the tests_using_key branch May 3, 2024 14:37
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.

4 participants