game-ci / unity-test-runner

Run tests for any Unity project
https://github.com/marketplace/actions/unity-test-runner
MIT License
206 stars 135 forks source link

Feature: Add support for Nuget dependency resolution when testing packages #266

Open nvandessel opened 4 months ago

nvandessel commented 4 months ago

Context

When testing a package that relies on a nuget package, resolving the dependency is painful. Currently the best work-around I've found is to simply include the .dll/s in the repo, which is not ideal for a number of reason. These include:

  1. Multiple packages needing same dependency, this makes this optional impossible as they .dll/s will clash.
  2. Updating them is a very manual process of replacing binaries
  3. Personally don't love having to store the binaries in my package repo

It would be nice if during the creation of a TempProject for adding the package under test to, that any defined nuget packages could also be resolved and included before importing the package. This is because the .dll/s need to be present before a package can be added to the project, otherwise the editor will fail to compile and as a result fail the CI.

This is really only a problem for packages, as it's likely that any project under test has the nuget .dll/s in the project already. Tools exist like NuGetForUnity which provide a nice mechanism for maintaining these files in a project.

Suggested solution

I believe we could simply leverage the dotnet cli, to download any required .dll/s and add them to the project before importing the package.

  1. In src/main.ts, add an argument to Docker.run called packageNugetDependencies.
  2. Additionally update the action.yml to have an optional configuration for packageNugetDependencies that can be used when in package mode.
  3. In run_tests.sh, once the TempProject has been created, but before the manifest is modified, check to see if any nuget dependencies are defined.
  4. If they are, the use the dotnet cli tool to download the .dll/s, and move them into the TempProject "Assets" directory
  5. Once this is completed, we should now be able to proceed as normal, modifying the manifest and re-opening the project

Considered alternatives

Another option would be to possibly use git-submodules and store the binaries in a separate repo. The package repo could then depend on the submodule and only resolve it when needed. This still does not really alleviate the pains of binaries now manually being modified and stored in a repo.

In addition, I've also already tried to use the aforementioned NuGetForUnity package. By adding it as a dependency to the package.json and trying to react to compilation, based on a suggestion from the maintainer there on this issue. Unfortunately the problem with this approach is that Unity detects the compile errors in the package before any of that code can execute, before the project even opens.

This does highlight another possible approach of, when the temp project is built, instead of using the dotnet cli to resolve the packages, the NuGetForUnity package could be added to the manifest of the temp project, along with it's required packages.config file that determines which Nuget packages and versions to resolve. I'm less partial to this idea though as it would require the engine to compile multiple times and places a direct dependency on NuGetForUnity.

Additional details

I'm really opening the issue to gauge any interest in this idea, and to possibly get some feedback on the general idea. I realize it may be a niche requirement and so thought it was best to open an issue first. If this idea is agreeable to the maintainers, it's something I'm willing to put some effort into implementing.

nvandessel commented 4 months ago

I guess thinking about it some more, another option would be to take the idea of having NuGetForUnity resolving the packages for me, and build an action for it. It could create a project, sets up the manifest with NuGetForUnity and define the packages.config file that it needs to resolve the .dlls. Then maybe this Project can be cached using the GitHub cache action, and passed through some argument to the game-ci action.

Then, ultimately all the game-ci action would need to change is the ability to receive a "ProjectDirectory" for package testing, which is a significantly smaller change with no added dependencies on any systems like nuget. In addition this could help resolve, although not directly, the problem of no caching support for package testing mentioned in this issue: #226

Kind of just riffing here, would appreciate any input on what sounds the most logical and reasonable.