Skip to content
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions src/NUnit.Engine.Api/ITestAgentProvider.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt

using System;

namespace NUnit.Engine.Services
{
/// <summary>
/// An object implementing ITestAgentProvider is able to provide
/// test agents, which satisfy the criteria specified in a TestPackage.
/// The engine loads implementations through /NUnit/Engine/AgentProviders.
/// Capability queries do not acquire execution ownership. An acquired agent
/// remains associated with its provider until ReleaseAgent succeeds.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would remove the sentence "The engine loads..." It sounds as if this the path is an extension point path, while it's just the relative location of the source code and not needed here. The "Capability queries statement could be moved to IsAgentAvailable or left here as you prefer. However, it needs to be qualified, perhaps in a "Remarks" section, perhaps in a separate document to which we refer, perhaps both.

Agents can be written by anyone. There is a mechanism to enquire regarding certain capabilities without instantiating the engine, provided the author uses that mechanism. In the early V4 code, that mechanism didn't exist. It was added later and the four existing V4 agents make use of it. However, the engine doesn't require that it be used and falls back to loading the extension into memory and instantiating the engine if the necessary attributes are not provided.

I realize that the above is not an adequate explanation, but let's decide where such an explanation should live and I'll write one.

/// </summary>
public interface ITestAgentProvider : ITestAgentInfo
{
/// <summary>
/// Returns true if an agent can be found, which is suitable
/// for running the provided test package.
/// </summary>
/// <param name="package">A TestPackage</param>
/// <returns>True if the provider can supply a compatible agent.</returns>
/// <remarks>Capability queries must not acquire execution ownership.</remarks>
bool IsAgentAvailable(TestPackage package);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See my comment above regarding agent behavior.


/// <summary>
/// Return an agent, which best matches the criteria defined
/// in a TestPackage.
/// </summary>
/// <param name="package">The test package to be run</param>
/// <returns>An ITestAgent</returns>
/// <exception cref="ArgumentException">If the package is invalid.</exception>
/// <exception cref="InvalidOperationException">If no compatible agent is available.</exception>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add something under InvalidOperationException to say you should check first using IsAgentAvailable I also think this needs a remark about ownership

ITestAgent GetAgent(TestPackage package);

/// <summary>
/// Releases the test agent back to the supplier, which provided it.
/// </summary>
/// <param name="agent">An agent previously provided by a call to GetAgent.</param>
/// <exception cref="InvalidOperationException">
/// If agent was never provided by the factory or was previously released.
/// </exception>
/// <remarks>
/// Return the agent through the same provider which acquired it so that
/// ownership bookkeeping remains consistent. A stop request, a closed
/// connection or disposal of a runner is not confirmation of release.
/// If release fails, retain ownership until the provider can reconcile it.
/// The provider defines how it stops its agent. Successful release alone
/// does not prove that external deployment files can be deleted safely;
/// remote providers must confirm execution has stopped before allowing cleanup.
/// </remarks>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Last clause applies to all providers.

void ReleaseAgent(ITestAgent agent);
}
}