Complete transition to pluggable agents - #1698
Conversation
|
It was difficult to get this all into a single commit, because I needed to publish a new release of the Agent Core package for use by the pluggable agents. That was done in PR #1697. The outcome of this PR is that agents are now entirely separate from the runner and are brought in as dependencies when the runner is installed. I think we may want to create a separate repo for the nunit-agent.core assembly, since it's not referenced any longer in this project. |
manfred-brands
left a comment
manfred-brands
left a comment
There was a problem hiding this comment.
The changes are mainly deletions.
Main issue is the compile time cyclic dependency between nunit-console and the agent repositories.
It almost makes sense to extract an engine repository containing the agent.core and engine nuget packages.
This repository then deals with nunit-console only, pulling in the engine and agents and gluing it all together.
There was a problem hiding this comment.
We might want to consider moving to the new .slnx file format.
Then we don't have to see all this guid = guid stuff.
From 9.0.200 SDK we can use dotnet sln migrate NUnitConsole.sln
There was a problem hiding this comment.
I didn't know about that. It looks very promising
| "nunit-agent-net80.dll", "nunit-agent-net80.dll.config", | ||
| "nunit.agent.core.dll", "nunit.extensibility.dll", "nunit.extensibility.api.dll", | ||
| "nunit.common.dll", "nunit.engine.api.dll", "testcentric.metadata.dll") | ||
| HasDependency("NUnit.Extension.Net462PluggableAgent", "4.0.0-alpha.6"), |
There was a problem hiding this comment.
Do we now have a cyclic build dependency where the agents repositories depend on the the nunit-console repository and the nunit-console depends on the agents?
There was a problem hiding this comment.
Only for breaking API changes. See my general comment.
There was such a dependency while I maintained both built-in and plug-in agents in the build, but that's over now.
There is still what I call a circular testing dependency. That is... proper engine testing needs at least one, preferably both, agents... agent testing requires a console build that supports that particular agent. I got around that temporarily by removing some tests.
This is where the direct agent execution feature we added earlier comes in. The engine tests can be run directly against agents and the agent tests can run without the console. Only the console package tests need the console itself now. I'll add the tests back in separately and just merge so long as they pass.
|
@manfred-brands Middle of the night comments to your comments... I'll make changes in the morning... Re cyclical dependency... In practice, based on my experience with TestCentric, it's less of a problem than you'd imagine. The extensions now only depend on the api and agent-core, and the correct copies are included in the agent package. Agent-core is only used in the agent process and isn't loaded at all by the engine. In fact, I think I can now remove it from the engine and runner packages entirely. (Was needed during the transition) The launcher is the only part loaded in the agent process, but it's of course in it's own directory. The agents will need to be rebuilt each time there's a breaking change to the API, which could cause a method not found exception but once we release with the final API, they should keep working. My main problem with the testcentric agents has been making changes when outside dependencies call for it. Mostly I handle those by dependabot. Even so, I think it would be useful to have all of them in a single solution. That solution could include the agent-core assembly as you suggest. We'd need to make two other changes, I think...
Neither of these would be needed immediately, although the versioning would be nice. Selective building is likely to come up only when one of the agents becomes outdated or if we add an out-of-support agent for V3 compatibility. I'll define an issue for this. |
|
@manfred-brands Turns out I can't completely remove agent-core from this repo yet. It contains the definition LocalTestRunner, used by the net80 build and for the drivers used by both builds. TestRunnerFactory depends on it. But that code doesn't need to be where it is, so I'll do another PR to move it. |
manfred-brands
left a comment
manfred-brands
left a comment
There was a problem hiding this comment.
Approved with the caveat about my concerns about the cyclic development dependency.
|
I think that creating a repo for all agents plus agent-core will handle that. I have to ask Terje to create it first and then I'll add an issue to list all the tasks needed to get it working. |
I wondered why you had 3 separate repositories for the agents. |
It' how I did it in TestCentric and I did it there as part of an overall experiment... i.e. what would happen if I only had one assembly being built per repository? So TestCentric is at the extreme with 30 repos inclding 10 agents. I learned a lot from that experiment and I've avoided going so far with the engine, but the versioning and archiving issues kept me from putting the agents together. So... let's solve those. :-) BTW, it turns out I don't need a new repo, I can just rename one of the two agent repos I just created since they are only published in our alpha feed. I'll add you to those repos and create issues in one or both of them. |
2 participants
Fixes #1673