Skip to content

Conversation

@m-redding
Copy link
Collaborator

@m-redding m-redding commented Sep 8, 2025

Onboarding openai-dotnet to Microsoft.ClientModel.TestFramework

General changes

  • Adds the test proxy to the local set of tools
  • Adds a reference to the dev feed version of Microsoft.ClientModel.TestFramework
  • Added an [Ignore] attribute to all tests that were failing consistently in main, after retrying enough times to be certain it wasn't just flaky
  • Migrated from NUnit 3 to NUnit 4, see https://docs.nunit.org/articles/nunit/release-notes/Nunit4.0-MigrationGuide.html for more information about the breaking changes

Test utility changes

  • Migrate from bespoke System.ClientModel mock types to use the mocks provided by Microsoft.ClientModel.TestFramework
  • Added an OpenAI test environment
  • Added an Open AI recorded test base:
    • Automatically handles proxying the client and options. Proxying the client is required for sync/async auto-conversion, that is how the automatic interception of async calls happens. Instrumenting the options is required for recorded tests, it injects the test proxy transport to allow for recording.
    • Adds additional sanitization

Mock test changes

  • Added auto async/sync conversion for tests and removed all logic for synchronous testing. This is done automatically by the test framework now. It will intercept any calls to asynchronous methods, and in sync mode, forwards them to the sync counterpart instead. It works for pageable APIs as well.
  • Moved some additional tests into mock test files since the recordings were empty (i.e. they didn't actually make any network calls), there's a bit more work to do on this front though

Recorded test changes

  • added auto async/sync same as with mock tests
  • Added recordings for live tests
  • Removed [Parallelizable] attributes - playback/record cannot be run in parallel due to constraints in NUnit, replaced with LiveParallelizable so that live test runs are still run in parallel. Since playback is significantly faster than live, this will really only slow down re-recording, which hopefully won't have to happen in bulk very often

Areas for future improvement

  • Realtime tests cannot be recorded yet due to issues with recording web sockets, need to fix these issues and then record these tests

@m-redding m-redding changed the title Use test framework Use auto sync/async from test framework Sep 8, 2025
@m-redding m-redding changed the title Use auto sync/async from test framework Onboard to test framework Sep 16, 2025
@m-redding m-redding requested a review from Copilot September 17, 2025 22:33
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR onboards the openai-dotnet library to Microsoft.ClientModel.TestFramework to modernize the testing infrastructure. The migration introduces test proxy support for HTTP recording/playback and standardizes async/sync test patterns.

  • Migrates from custom mock implementations to Microsoft.ClientModel.TestFramework mocks
  • Adds test proxy integration for HTTP recording/playback with test session records
  • Implements automatic sync/async test conversion eliminating manual synchronous test logic

Reviewed Changes

Copilot reviewed 59 out of 580 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/SessionRecords/AssistantsTests/FunctionToolsWork.json New test recording for function tools functionality test
tests/SessionRecords/AssistantsTests/FileSearchWorksAsync.json New test recording for async file search functionality
tests/SessionRecords/AssistantsTests/FileSearchWorks.json New test recording for file search functionality
tests/SessionRecords/AssistantsTests/FileSearchStreamingWorksAsync.json New test recording for async streaming file search
tests/SessionRecords/AssistantsTests/FileSearchStreamingWorks.json New test recording for streaming file search functionality
tests/SessionRecords/AssistantsTests/BasicThreadOperationsWorkAsync.json New test recording for async thread operations
tests/SessionRecords/AssistantsTests/BasicThreadOperationsWork.json New test recording for basic thread operations

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@m-redding m-redding marked this pull request as ready for review September 17, 2025 22:41
Copy link
Collaborator

@jsquire jsquire left a comment

Choose a reason for hiding this comment

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

I didn't scrutinize each test class, but poking through randoms look like the expected patterns. We'll want to get some additional eyes on, but I think it's pretty low risk to advance this, as the tests aren't super-reliable today.

@m-redding
Copy link
Collaborator Author

Just pushed commits that:

  • re-recorded multipart tests to use the new functionality from the test proxy
  • added recordings for tests that were previously ignored due to org verification
  • added an "api-key" placeholder in the variables to fix some issues in playback.
  • added recordings for recently merged tests

Tests are all green - lmk if there's any additional feedback!!

@m-redding m-redding merged commit 65d5703 into openai:main Sep 29, 2025
1 check passed
@m-redding m-redding deleted the addtestframeworktake2 branch September 29, 2025 17:45
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