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 setApiKey method #157

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

badasswp
Copy link

@badasswp badasswp commented Feb 6, 2025

This PR resolves this issue.

Description / Background Context

At the moment, it is difficult to effectively mock our OpenAi class in unit tests because we are currently forced to pass in the API key as an argument during instantiation.

This PR creates a new setApiKey method that enables us do this anytime so that we are not restricted to the constructor. In this way, we can create and pass our objects via dependency injection like so:

interface AiClient
{
    public function chat($payload): void;
}

public function getAiClient(AiClient $aiClient)
{
    return $aiClient;
}

public function getOpenAiClient()
{
    $this->getAiClient(new OpenAi())->setApiKey($keys);
}

In this way, we should be able to easily mock our OpenAi class for use in unit tests like so:

public function testOpenAiDoesSomething()
{
    $openAiMock = Mockery::mock(OpenAi::class)->makePartial();
    // write your other test logic....
}

Testing Instructions

  1. Pull PR to local.
  2. Run composer install.
  3. Run unit tests: ./vendor/bin/pest --group=api-keys
  4. Tests should pass successfully like so:
open-ai

Closes: #156

@badasswp badasswp changed the title Add set api key method Add setApiKey method Feb 6, 2025
@badasswp
Copy link
Author

badasswp commented Feb 7, 2025

Hi @orhanerday @pascalbajorat If any of you have got some time later today, could you pls review this PR. Thanks.

@orhanerday
Copy link
Owner

Hey @badasswp,
I’m pretty busy these days with my role upgrade at the company. I’ll check it as soon as I can, but I can’t give an exact time. Thanks for your patience!

@badasswp
Copy link
Author

badasswp commented Feb 7, 2025

Thanks @orhanerday. I appreciate 🙏

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.

Add setApiKey method
2 participants