Introduce IServiceLocator.TryGetService API method

[CharliePoole]

Fixes #897

[Copilot]

Pull Request Overview

This PR introduces a new TryGetService<T> API to complement GetService<T>, updates internal service lookup to return null instead of throwing, and adjusts callers and tests to use the new pattern.

• ServiceManager.GetService now returns IService? and logs errors instead of throwing.
• ServiceContext and IServiceLocator added a generic TryGetService<T>; tests updated to cover both GetService<T> and TryGetService<T>.
• Console runner and Cake script refactored to use the new TryGetService<T> and reorganized package push logic.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/NUnitEngine/nunit.engine/Services/ServiceManager.cs	Changed GetService(Type) to return nullable and defer exception handling
src/NUnitEngine/nunit.engine/Services/ServiceContext.cs	Added TryGetService<T> and updated GetService<T> to throw on null
src/NUnitEngine/nunit.engine.tests/Services/ServiceManagerTests.cs	Adapted tests to nullable return and used null‐propagating operators
src/NUnitEngine/nunit.engine.tests/Api/ServiceLocatorTests.cs	Added tests for generic GetService<T> and TryGetService<T>
src/NUnitEngine/nunit.engine.api/IServiceLocator.cs	Extended interface with TryGetService<T> and updated documentation
src/NUnitConsole/nunit4-console/Program.cs	Switched to TryGetService<T> for runtime lookup and restructured error branches
cake/publishing.cake	Moved package name/path creation into each branch and simplified push logic

Comments suppressed due to low confidence (2)

src/NUnitEngine/nunit.engine.api/IServiceLocator.cs:26

• Add XML documentation for the generic <typeparam name="T"> and <param name="service"> on TryGetService<T> to clarify its contract and nullability behavior.

        bool TryGetService<T>(out T? service)

src/NUnitEngine/nunit.engine/Services/ServiceManager.cs:25

• Update the XML doc for GetService(Type) to indicate that it may return null instead of throwing, matching its new signature.

        public IService? GetService(Type serviceType)

[Copilot]

[nitpick] The null check availableRuntimes is not null is redundant, since TryGetService<T> already guarantees availableRuntimes is non-null when it returns true.

Suggested change

	if (engine.Services.TryGetService<IAvailableRuntimes>(out var availableRuntimes) && availableRuntimes is not null)
	if (engine.Services.TryGetService<IAvailableRuntimes>(out var availableRuntimes))

[CharliePoole]

I had thought it should work without the check for not null, but it didn't. I have a general problem with this pattern. Thoughts?

[veleek]

Adding the not null when attribute would allow this to work. Copilot seems to be assuming that's the behavior? Maybe it's biased but all the built in .net code that has those attributes on TryX methods?

[CharliePoole]

Except I already have the attribtue on the TryGetNull out argument. I assume that's why Copilot thinks it's guaranteed.

@manfred-brands I learned the pattern of double-checking from some of your code. Do you know when exactly it's needed and when not?

[manfred-brands]

@CharliePoole You don't have the attribute on the interface, so calls using the interface don't know there is only one implementation. Adding the attribute there allows us to remove the extra null check.

Not sure which double checking you mean.
The only double checking would be inside a lock where potentially the previous code could have set the variable.

[CharliePoole]

Ah! The interface. Of course! I'm still not really comfortable with the static analysis having come up in a world where pretty much anything could turn out to be null, i.e. assembly language and C. :-)

I can't point to the code but you used the same pattern if (Tryxxx && something is not null) a few times when you added in nullability checks to the codebase. I think I remembered it because it was unusual. Could have been that you didn't want to or couldn't change an interface.

Thanks for the fix.

[CharliePoole]

@manfred-brands I requested a copilot review in addition to see how it works. I'll comment on it's comments but wait for you before I actually make changes.

[veleek]

Saw the description by copilot describing the behavior of GetService as the opposite of what it actually is. 😂

ServiceManager.GetService now returns IService? and logs errors instead of throwing.

[manfred-brands]

I added a commit to add the NotNullWhen attribute on the IServiceLocator interface and removed the double check in Program.cs

The only comment besides the ones from Co-pilot is the name of ServiceManager.GetService which doesn't convey it could return null.

[manfred-brands]

Thanks. Nothing more from me.