enhance tests

[sikutisa]

Purpose

• Closes Tests are NOT testing exceptions #451
• Exception에 대한 테스트 코드 수정
• 테스트 코드 추가

Does this introduce a breaking change?

[ ] Yes
[x] No

Pull Request Type

What kind of change does this Pull Request introduce?

[ ] Bugfix
[ ] New feature
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[x] Other... Please describe:

README updated?

The top-level readme for this repo contains a link to each sample in the repo. If you're adding a new sample did you update the readme?

[ ] Yes
[x] No
[ ] N/A

Other Information

• HuggingFaceConnectorTests의 Given_Valid_Settings_When_GetChatClient_Invoked_Then_It_Should_Return_ChatClient를 Unit Test로 변경하고 싶은데, 마땅한 방법이 안 떠오르네요

[justinyoo]

최신 브랜치로 현행화하면 GitHubModelsConnectorTests, HuggingFaceConnectorTests, OpenAIConnectorTests 의 변경사항을 볼 수 있어요.

Ollama, HuggingFace, LG 커넥터에서 GetChatClient 메소드는 아마 단위테스트가 안 될 겁니다. 의존성 개체를 외부에서 주입하는 것이 아니라 내부에서 생성하기 때문입니다. 최신 브랜치로 현행화 하면 관련 내용을 확인할 수 있을 거예요.

[sikutisa]

일단 Abstraction 테스트들 먼저 수정하고, 이후에 Main에 새로 추가된 Connector들 테스트도 같이 보겠습니다.

[justinyoo]

두 가지

1. ArgumentOptionsTests와 LanguageModelSettingsTests를 보면 아래와 같은 부분이 있습니다.

public void Test_Method(Type type)
{
    // Act
    var isSubclass = type.IsSubclassOf(typeof(ArgumentOptions));

    // Assert
    isSubclass.ShouldBeTrue();
}

다른 테스트와 마찬가지로 IsAssignableFrom() 메소드로 바꿉시다.

public void Test_Method(Type derivedType)
{
    // Arrange
    var baseType = typeof(ArgumentOptions);

    // Act
    var result = baseType.IsAssignableFrom(derivedType);

    // Assert
    result.ShouldBeTrue();
}

2. 쭉 보다 보니... 맞춰야 할 것이 있습니다. 일구님은 아마 아래 처럼 하고 싶은 모양인 것 같아요.

public async Task Test_Method_Name()
{
    ...
    // Act
    Func<Task> func = XxxxAsync();

    // Assert
    var ex = await func.ShouldThrowAsync<T>();
    ex.Message.ShouldContain(...);
}

근데 아래처럼 하는 것이 더 낫습니다. 이유는 Fluent API를 바로 쓸 수 있기 때문인데요, 위처럼 하면 일단 ex를 한 번 받아와야하는 상황이 생깁니다. 아래처럼 Fluent API 방식을 도입하면 조금 더 테스트 결과 확인이 계속 하나의 컨텍스트 안에서 이어지니 좀 더 수월할 거예요.

public void Test_Method_Name()
{
    ...
    // Act
    Func<Task> func = async () => await XxxxAsync();

    // Assert
    func.ShouldThrowAsync<T>()
        .Message.ShouldContain(...);
}

어떻게 생각하세요?

[justinyoo]

@sikutisa 커넥터가 하나둘씩 머지될 때 마다 범위가 넓어집니다. 후다닥 해치우시죠?

[sikutisa]

@sikutisa 커넥터가 하나둘씩 머지될 때 마다 범위가 넓어집니다. 후다닥 해치우시죠?

1. 전체적인 수정 사항을 요약하자면 아래와 같습니다.

• ArgumentOptions에 대한 상속 테스트를 각 구현체 테스트로 옮겼습니다.
• LanguageModelSettings의 상속 테스트의 경우, 각 구현체에 대한 테스트가 없기 때문에 내용만 수정했습니다.
• LanguageModelConnectorTest
  • 테스트가 추가됐습니다.
  • Exception 테스트 코드 변경했습니다.
  • Inline 데이터 추가했습니다.
  • LanguageModelConnector에 대한 상속 테스트를 각 구현체 테스트로 옮겼습니다.
• 각 커넥터 테스트
  • Exception 테스트 코드 변경했습니다.
  • Inline 데이터 추가했습니다.
  • 테스트가 추가됐습니다.
  • 테스트 순서를 일관되게 재정렬했습니다.
    • 상속테스트
    • 커넥터 객체 생성 실패
    • 커넥터 객체 생성 성공
    • 케이스별 EnsureLanguageModelSettingsValid 실패
    • EnsureLanguageModelSettingsValid 성공
    • 케이스별 GetChatClientAsync 실패
    • GetChatClientAsync 성공
    • 케이스별 CreateChatClientAsync 실패
    • CreateChatClientAsync 성공

2. HuggingFace, LG 작업 전에 질문이 있습니다. PullModelAsync의 존재로, GetChatClientAsync와 CreateChatClientAsync에 대한 테스트를 하기 위해서는 LLMRequired 로 작성해야 할 것 같은데 기존에 작성된 테스트가 거의 없는 것 같습니다. 다른 커넥터와 동일한 테스트를 LLMRequired로 작성하면 되는지, 아니면 이 부분은 홀드하기로 했는데 제가 놓치고 있는 건지 여쭤봅니다.

[justinyoo]

몇군데 false alarm 으로 자동수정된 부분들 수정하는 거랑 기존에 수정 요청 놓쳤던 거랑 해 보시면 될 듯 합니다.

[sikutisa]

1. HuggingFace와 LG 커넥터 테스트 수정했습니다.

• 일부 테스트 추가 했습니다.
• LLM 사용이 필요한 테스트 재분류 했습니다.
• 일부 테스트 Inline data를 수정했습니다.

2. false alarm 원복했습니다.

[justinyoo]

LGTM 수고하셨습니다!