Skip to content

Fix SharedCodeCompletionProviderTests assembly loading for completion data tests#16889

Open
Copilot wants to merge 4 commits intomove_python_targetfrom
copilot/sub-pr-16760
Open

Fix SharedCodeCompletionProviderTests assembly loading for completion data tests#16889
Copilot wants to merge 4 commits intomove_python_targetfrom
copilot/sub-pr-16760

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 11, 2026

The test SharedCoreCanReturnCLRCompletionData was failing with ArgumentNullException: Value cannot be null. (Parameter 'source') due to assembly loading issues that prevented SharedCompletionProvider.GetCompletionData() from finding the required implementation.

Changes

  • Added [SetUp] attribute to SharedCodeCompletionProviderTests.Setup() method to ensure NUnit executes the setup code
  • Fixed assembly loading by directly referencing DSPythonNet3CodeCompletionProviderCore type instead of path-based loading
  • Added null check assertion for GetCompletionData() result with descriptive error message

Root Cause

The SharedCompletionProvider uses reflection to find types implementing IExternalCodeCompletionProviderCore in loaded assemblies. The original code attempted to load the assembly from a file path using PathManager.BuiltinPackagesDirectory, which doesn't exist or isn't properly configured during test execution. Without the assembly loaded in the AppDomain, the provider couldn't find the implementation and returned null.

Solution

[TestFixture]
internal class SharedCodeCompletionProviderTests : UnitTestBase
{
    [SetUp]  // <- Added this attribute
    public override void Setup()
    {
        base.Setup();
        // Force loading of the DSPythonNet3 assembly by accessing a type from it
        // This ensures the assembly is in AppDomain.CurrentDomain.GetAssemblies()
        // before SharedCompletionProvider tries to find IExternalCodeCompletionProviderCore implementations
        var _ = typeof(DSPythonNet3CodeCompletionProviderCore);
    }
}

By referencing the type directly, the CLR loads the assembly since the test file already imports it with using DSPythonNet3;. This approach is more reliable than path-based loading because:

  • No dependency on PathManager.BuiltinPackagesDirectory being correctly configured for tests
  • No file system access needed
  • Works with NuGet package references
  • Simpler and more maintainable code

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: QilongTang <3942418+QilongTang@users.noreply.github.com>
Copilot AI changed the title [WIP] Move Python test out of core for Dynamo Add missing [SetUp] attribute to SharedCodeCompletionProviderTests.Setup() Feb 11, 2026
Copilot AI requested a review from QilongTang February 11, 2026 21:40
@QilongTang QilongTang marked this pull request as ready for review February 11, 2026 21:57
@QilongTang
Copy link
Copy Markdown
Contributor

@copilot The target test is still failing with the same error message

Copilot AI and others added 2 commits February 12, 2026 14:04
Co-authored-by: QilongTang <3942418+QilongTang@users.noreply.github.com>
Co-authored-by: QilongTang <3942418+QilongTang@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Feb 12, 2026

@copilot The target test is still failing with the same error message

The issue was that the DSPythonNet3 assembly wasn't being loaded before SharedCompletionProvider tried to find IExternalCodeCompletionProviderCore implementations. Fixed in 7e49d69 by directly referencing the type (typeof(DSPythonNet3CodeCompletionProviderCore)) which forces the CLR to load the assembly, rather than trying to load from a path that may not exist during test execution.

Copilot AI changed the title Add missing [SetUp] attribute to SharedCodeCompletionProviderTests.Setup() Fix SharedCodeCompletionProviderTests assembly loading for completion data tests Feb 12, 2026
@sonarqubecloud
Copy link
Copy Markdown

@QilongTang
Copy link
Copy Markdown
Contributor

@QilongTang
Copy link
Copy Markdown
Contributor

@copilot Rebase

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 9, 2026

@copilot Rebase

Rebase completed successfully onto master (fee9456). The branch has been rebased with all commits preserved:

  • 4f8816a3df: move target for unzipping python package
  • ff3bc3aadc: Revert "move target for unzipping python package"
  • c4a2a05969: move the test (resolved merge conflict in PackageDependencyTests.cs)
  • dd6a42e3db: Initial plan
  • 4b5cbb9dd3: Fix test failure: Add [SetUp] attribute to Setup method
  • c5bc56f722: Add diagnostic checks for assembly loading and null completion data
  • b0687385ba: Fix assembly loading by referencing type directly

Unable to force push due to permissions. Please manually force push the branch or grant push permissions.

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