Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR removes custom TUnit skip attributes so that previously integration-gated C# tests execute in CI, simplifying the test suite and increasing CI coverage for end-to-end scenarios.
Changes:
- Deleted
SkipUnlessIntegrationAttributeand its unit tests, plus the unusedSkipInCIAttribute. - Removed
[SkipUnlessIntegration]from several integration/E2E test classes and methods to allow them to run in CI.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| sdk/cs/test/FoundryLocal.Tests/SkipUnlessIntegrationTests.cs | Removed tests for the deleted integration-skip attribute. |
| sdk/cs/test/FoundryLocal.Tests/SkipUnlessIntegrationAttribute.cs | Deleted custom conditional skip attribute. |
| sdk/cs/test/FoundryLocal.Tests/SkipInCIAttribute.cs | Deleted unused CI-skip attribute. |
| sdk/cs/test/FoundryLocal.Tests/LiveAudioTranscriptionTests.cs | Unskipped session-guard and E2E streaming tests. |
| sdk/cs/test/FoundryLocal.Tests/FoundryLocalManagerTest.cs | Unskipped manager/catalog integration tests. |
| sdk/cs/test/FoundryLocal.Tests/EndToEnd.cs | Unskipped the end-to-end test that can mutate the model cache. |
| sdk/cs/test/FoundryLocal.Tests/ChatCompletionsTests.cs | Unskipped chat completions integration tests. |
| sdk/cs/test/FoundryLocal.Tests/AudioClientTests.cs | Unskipped audio transcription integration tests. |
Comments suppressed due to low confidence (5)
sdk/cs/test/FoundryLocal.Tests/ChatCompletionsTests.cs:19
- With [SkipUnlessIntegration] removed, this class now always runs but its Setup immediately uses FoundryLocalManager.Instance (which throws unless Utils.AssemblyInit successfully called FoundryLocalManager.CreateAsync). If integration infrastructure isn’t available, the resulting failure won’t include the LOCAL_MODEL_TESTING.md guidance that the old skip reason provided. Add a guard in Setup to skip/fail with clear setup instructions when Utils.IntegrationTestsAvailable is false.
internal sealed class ChatCompletionsTests
{
private static IModel? model;
sdk/cs/test/FoundryLocal.Tests/FoundryLocalManagerTest.cs:18
- Now that [SkipUnlessIntegration] is removed from the whole class, Manager_GetCatalog_Succeeds will run even when the integration test setup didn’t initialize FoundryLocalManager, causing FoundryLocalManager.Instance to throw a generic “Call CreateAsync first” error. Consider guarding these tests (skip/fail with LOCAL_MODEL_TESTING.md instructions) based on Utils.IntegrationTestsAvailable so the failure mode is clearer and doesn’t depend on environment quirks.
public class FoundryLocalManagerTests
{
[Test]
public async Task Manager_GetCatalog_Succeeds()
{
sdk/cs/test/FoundryLocal.Tests/LiveAudioTranscriptionTests.cs:139
- These tests now run without [SkipUnlessIntegration], but LiveAudioTranscriptionSession accesses FoundryLocalManager.Instance in field initializers, so constructing the session will throw unless Utils.AssemblyInit successfully initialized the manager. That means the tests can fail before reaching the intended “before StartAsync” guard assertions. Add an explicit integration-availability guard at the start of these tests (or construct the session via an initialized model’s audio client) so failures are deterministic and actionable.
[Test]
public async Task AppendAsync_BeforeStart_Throws()
{
await using var session = new LiveAudioTranscriptionSession("test-model");
var data = new ReadOnlyMemory<byte>(new byte[100]);
sdk/cs/test/FoundryLocal.Tests/LiveAudioTranscriptionTests.cs:158
- Same issue here: without [SkipUnlessIntegration], creating LiveAudioTranscriptionSession can throw due to FoundryLocalManager.Instance access during construction, so the test may fail before exercising GetTranscriptionStream’s pre-start guard. Add an integration availability guard (or create the session from an initialized audio client) to ensure the test validates the intended behavior.
[Test]
public async Task GetTranscriptionStream_BeforeStart_Throws()
{
await using var session = new LiveAudioTranscriptionSession("test-model");
sdk/cs/test/FoundryLocal.Tests/EndToEnd.cs:16
- This test is described as “run manually as a standalone test as it alters the model cache,” but removing [SkipUnlessIntegration] will make it run in normal CI/test runs. Either update the comment to reflect the new intent, or reintroduce an explicit guard (skip/trait/category) so it doesn’t unexpectedly run in environments where cache mutation/downloads are undesirable.
internal sealed class EndToEnd
{
// end-to-end using real catalog. run manually as a standalone test as it alters the model cache.
[Test]
public async Task EndToEndTest_Succeeds()
{
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
C# test cleanup: