-
Notifications
You must be signed in to change notification settings - Fork 153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Isolate the agents from the engine #1619
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good as far as I can see on phone.
A few TODOs.
<ExposedInternals>true</ExposedInternals> | ||
</PropertyGroup> | ||
|
||
<ItemGroup Condition="'$(TargetFramework)'=='net462' OR '$(TargetFramework)'=='net6.0' OR '$(TargetFramework)'=='net8.0'"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no net6.0
target anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup. I'll make that change but wait to see what else you have. You may also notice that there are untested agent-core targets: netcoreap3.1 and net6.0. I keep them because I know I'll use them later for separate user-installable agents. I see no point of doing it for anything older than those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left it in for now as it does no harm.
@@ -21,9 +21,11 @@ | |||
</ItemGroup> | |||
|
|||
<ItemGroup> | |||
<ProjectReference Include="..\..\Extensibility\nunit.extensibility\nunit.extensibility.csproj" /> | |||
<ProjectReference Condition="'$(TargetFramework)'=='net8.0'" | |||
Include="..\..\NUnitCommon\nunit.agent.core\nunit.agent.core.csproj" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Engine references agent? But only for .net8.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes... because the .NET 8.0 engine only runs in process, making it a sort of agent in itself. I want to eventually create an in-process agent, at which point we can drop the reference.
Fixes #1578