Skip to content
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

Simplify locating of addins #488

Open
nvborisenko opened this issue Oct 18, 2018 · 34 comments
Open

Simplify locating of addins #488

nvborisenko opened this issue Oct 18, 2018 · 34 comments

Comments

@nvborisenko
Copy link
Contributor

Currently there is only one way how user is able to say where nunit-console should find extensions. This is via *.addins file. What if user will be able to install extension via nuget into tests project and that's all what is required from user.

I propose to extend nunit-console and try to search extensions in the directory where user's tests dll is located. It would be default behavior.

@CharliePoole
Copy link
Collaborator

You can currently do this by either modifying the delivered addins file or (better) adding a new one pointing to that directory.

That may not totally do the job for you but if you try it I think we'll learn better what us needed.

@ChrisMaddock
Copy link
Member

Where is your console coming from at the moment? If you install both console and addins by NuGet, the engine should find the addins automatically already.

@nvborisenko
Copy link
Contributor Author

Imaging nunit-console came from unknown location, for instance it's pre-installed on build machine (appveyour, etc).

And I do

nunit3-console /full/path/to/my/tests.dll

Actually I don't know where exe is located, hence I cannot use *.addins file.

Generally if you explore some example of the extension https://github.com/reportportal/agent-net-nunit/blob/master/README.md, you might see that the hardest part of the installation is to say nunit-console how to find the extension.

This issue is about the following case:

  1. User developed tests
  2. Installed extension via nuget, built/publish project
  3. Copied all artifacts to some another machine
  4. Executed tests

@CharliePoole
Copy link
Collaborator

It's probably the biggest issue with extensions that there's no easy way to install them manually.

If you use either nuget or (my favorite) chocolatey, they are all findable very easily. But if you copy artifacts, you are basically down to a manual process, which is easy to get wrong.

As you can see from the docs, the process of locating extensions starts wherever the nunit.engine.dll is located. If it is located in your project, because you installed via nuget.org, then any extensions that are also installed by nuget.org are found automatically.

When I use chocolatey to install a copy of the console runner centrally, then the starting point is inside a chocolatey directory. Extensions that are also installed via chocolatey are found, but no others.

When the msi is installed centrally, there is no way to add new extensions except to add new addin files in the the engine directory.

This overall separation of different install methods is actually by design. If you install the engine in your project, it should not be picking up extensions from some central location. However, IMO at least, it would make sense to do as you are asking and have the central installs also look into the project that is being executed for extensions.

@ChrisMaddock
Copy link
Member

Hey @nvborisenko - you've raised a good point. I agree that in your situation there should be an easier way to install extensions. I'm less convinced that searching the test directory for them is the right solution.

As far as I can see, there's a few drawbacks to that approach:

  1. The engine will now have to examine every assembly in the test directory to look for extensions. In the case of big projects, that's a lot of assemblies, and may be a significant time cost - especially in the case of running a single test.
  2. It feels like a bit of a semantic oddity to me. Why does something I have as a dependency of my test project affect how the test runner is working?
  3. What happens if I pass two assemblies into the console runner - with an extension in the test directory of only one of these assemblies? Do all extensions apply to all assemblies? Are the extensions enabled and disabled depending which assembly they were found besides? (Would the latter even make technical sense?)

Would a console command line parameter be a better solution? We already have --list-extensions - how about a --load-extensions? This could take a file path to either a directory, extension assembly, or addins file, to keep it consistent with current extension loading functionality.

Tagging in @nunit/engine-team for their thoughts - let's see what others think about this before going any further. 🙂

@jnm2
Copy link
Collaborator

jnm2 commented Oct 20, 2018

It feels like a bit of a semantic oddity to me. Why does something I have as a dependency of my test project affect how the test runner is working?

I've always felt this way about referencing the VSTest adapter from a csproj. :D

--extensions-dir maybe? I like the concept and I agree that we don't want to be scanning the project output being tested.

@ChrisMaddock
Copy link
Member

I've always felt this way about referencing the VSTest adapter from a csproj. :D

Yes - same!


--extensions-dir maybe? I like the concept and I agree that we don't want to be scanning the project output being tested.

And limit it just to take a directory? It seems like an overhead to need to create a separate directory specifically to house a single extension assembly. I think it would be good to also support the .addins file, to be consistent current behaviour.

@jnm2
Copy link
Collaborator

jnm2 commented Oct 20, 2018

Oh, true. --load-extensions is better for that. Would it ever be beneficial to disable the default loading search logic? I can't think of a reason.

@CharliePoole
Copy link
Collaborator

The search for extensions starts in a root directory, which is the directory containing the engine itself.
The idea was that this might be expanded to search in other directories as well, using exactly the same search pattern. One possibility is a common directory used by all engine extensions. Another is some directory specific to the test project being loaded.

However, if you are loading multiple assemblies, there is still only one engine with extensions (i.e. the primary engine) It would take a fair bit of effort, I think, to load an extension only for one assembly, possibly a full restructuring of how we load assemblies. For that reason I agree with an approach where the user simply tells the user where to look for extensions. The command-line makes sense for this purpose. All the console would need to do would be to add the new setting to the TestPackage and the engine should do the rest.

@ChrisMaddock
Copy link
Member

Great! @nvborisenko - does the command line option still work for you, and are you still interested in implementing it?

I suggest --load-extensions can take an extension assembly, a directory, or a .addins file. I think it should be possible to use --load-extensions multiple times, for future-proofing. The load-extensions options will need to go into a new EnginePackageSetting, which can be passed into the engine, and the ExtensionService can then load any relevant extensions.

@CharliePoole
Copy link
Collaborator

@ChrisMaddock That flexibility (dll/addins/directory) is probably useful for the user even if it takes a bit more work to reorganize the service initialization code. It seems doable.

You haven't specified what a directory means. I suggest that it should mean the same thing as if you added the directory path to the addins file. That is: process any .addins files if present OR process *.dll in that directory. This is slightly different from how we process the original engine directory, where we ignore any dlls, but it seems less surprising.

@ChrisMaddock
Copy link
Member

Would it ever be beneficial to disable the default loading search logic? I can't think of a reason.

Me neither. Let's cross that bridge when we come it it. 🙂 There's workarounds, like adding an empty. .addins file.

@ChrisMaddock
Copy link
Member

You haven't specified what a directory means. I suggest that it should mean the same thing as if you added the directory path to the addins file. That is: process any .addins files if present OR process *.dll in that directory. This is slightly different from how we process the original engine directory, where we ignore any dlls, but it seems less surprising.

Good point - I agree with what you say!

@CharliePoole
Copy link
Collaborator

If we want to run without any extensions I can imagine a --no-extensions option in future. It's possible to enable and disable extensions, but we haven't exposed the possibility in the runner.

@nvborisenko
Copy link
Contributor Author

Command line option can be considered as working solution. Fifty cents from me:

  • --load-extensions looks like boolean option, load or not. It's not clear whether applying this option overrides default *.addins behavior, will they work together?
  • Multiple usage: is --load-extensions path1;path2 better than--load-extensions path1 --load-extensions path2?

New idea came: what if engine searches for "*.addins" files in test directory and processes them as usual. Hence user needs create addins file in test project and specify relative path to an extension. This resolve the issue when user doesn't know where exactly nunit-console.exe is located, he knows that an extension will be loaded by runner. This way resolves my initial issue with complicated installation of extension. Moreover, nuget package of extension can add addins file automatically. So user need just install nuget package and that is all, extension will be loaded by engine.

@goblinmaks
Copy link

Is it will be implemented in soon time ?

@CharliePoole
Copy link
Collaborator

We have discussed but not yet made any decision and nobody has taken the task.

@nvborisenko
Copy link
Contributor Author

After some time passed, I am returning back. Actually for me it was enough to hear about that "engine starts look up *.addins file in the folder where it is located". It allowed me create addins file and specify where my addin.dll is located. Particular it worked with NUnit3TestAdapter: Visual Studio loads adapter from "bin\Debug" folder, engine.dll is located there too, and my addin.dll is there too.

Using this approach was enough to me. But recently Visual Studio decided to load adapter from nuget package folder, instead of "bin\Debug". As a result approach stopped to work. I lost the actual folder where engine.dll is located. It might be in "packages/NUnit3TestAdapter/*" or in nuget global cache folder. So I don't know where to create addins file.

I understand that extension service should be totally redesigned. What if engine.dll considers Environment.CurrentDirectory to start discovering addins? CurrentDirectory is more predictable directory. In case of NUnit3TestAdapter the directory *.sln directory.

Easy code change - big profit.

@CharliePoole
Copy link
Collaborator

The nuget install of NUnit should include a .addins file, which works with extensions installed via nuget. Can you do something with that?

It seems to me that CurrentDirectory works for the adapter, at least until VS changes the directory, but doesn't work well generally. For example I can run a command like...

..\..\nunit3-console some\directory\mytest.dll

OTOH I've often wanted a way to have NUnit detect that it's running as part of a VS project and use additional addins locations in that case.

@nvborisenko
Copy link
Contributor Author

NUnit3Adaper comes with addins file, by default it includes all "near" nuget packages.

Problems:

  • It's unclear what is version of nuget package is picked up by engine
  • Addin package has to ship all dependencies inside to be loaded correctly

Let me remember the topic of this issue: simplify locating of addins. I want to develop nunit extension (which is delivered as nuget package because of this is the easiest way to deliver something in .net world). User is free to install this nuget package into tests project, and nunit engine should use this package as addin. Any special requirements for package?..

To be more specific: I want to develop "html reporter". User has a test project, user installs "html test reporter" into his test project via nuget - and he gets html report in all ways he executes tests (via console, via adapter). Is it possible?

@CharliePoole
Copy link
Collaborator

I agree that a simplified approach is a good idea. I just don't think the Current Directory is reliable in all cases since the user can be in any directory and still run NUnit3-console.

At least for the console, it seems to me that the application base directory would be more reliable. Not sure how that would work for the adapter, however.

@ChrisMaddock
Copy link
Member

ChrisMaddock commented Jan 24, 2020

Ooh, glad to see people interested in this issue - it's something I've wanted to look at for a while!

My thoughts: the engine itself provides a simple interface to locate extension. How to make that more user-friendly is a situation which is different per runner - any my opinion is that any changes we want to make here would be to the runners, (i.e. the adapter, or console).

For example:

  • Based on your comments, Nikolay, the adapter may want a different default path, set to wherever NuGet packages installed into a particular project end up. This would be achieved through pacaking (or dynamically modifying) the addins file. (Or perhaps via direct integration with the ExtensionService.) Either way - this would possibly rely on information only available in the adapter.
  • For the NUnit Console, I've long since dreamed of an nunit3-console install-addin path/to/myaddin.dll type of command
  • A GUI runner such as ReSharper/TestCentric may just wish to instead just pop up a file explorer window, let the user select the relevant assembly, and append it to the addin's file.

For this particular GitHub issue - I'd love to see someone take on a console command such as the above, but I don't (yet!) think any change to the engine is necessary. On the adapter - I'd defer to @OsirisTerje, and suggest a separate issue over in that repo to look at the adapter implementation specifically. 🙂

@ChrisMaddock
Copy link
Member

All that said, the point of @nvborisenko's I haven't touched on is being able to install a dependency and have that add-in apply to runs extensions of that test.

I'm not sure on this - it's sounds potentially costly performance wise. Remember the engine and extensions are all invoked whenever you run a single test in the adapter/ReSharper. It sounds like it could be a little heavy to investigate every reference of the test assembly for possible NUnitExtensionAttribute's before running a single test every time...but I haven't tried it in practice...

@CharliePoole
Copy link
Collaborator

Adding a small point to the mix here...

My expectation was that folks would not modify the provided addins file but would add additional files, perhaps one file per extension, in the same directory. That's why we initially start the search for extensions with a directory rather than a single file.

@CharliePoole
Copy link
Collaborator

A couple of ideas for introduction of new extensions more easily...

  1. Provide a new method for adding new directories to the initial one that is searched. This would need to be added to the extension service interface. I already did something similar for adding assemblies to be searched for extension points. (not sure if it's in the NUnit engine, however)

  2. Specify either "NUnitExtensionDir" or "NUnitExtensionPath" as an environment variable. "Dir" would be one directory. "Path" could be multiple semicolon-separated directories. Installing a new extension would simply involve dropping a .addins file into a directory. This seems ultra simple to implement and wouldn't change any interfaces. However, if the engine is checking this variable, it will mean that all runners get the same extensions. I don't know if that's a feature or a bug!

@CharliePoole
Copy link
Collaborator

Here's number 3, which is merely a possible workaround...

  1. Find the primary .addins file for the console on your machine. Add another .addins file in the same place with a single line that points to a directory where you will drop the .addins files for additional extensions. The engine will find those extensions. What I don't know is how this will work across nuget restores or on your CI machines.

@nvborisenko
Copy link
Contributor Author

My expectations:

  • Extensions should NOT be machine scoped. So we don't want to use environment variables, or any other global settings. We don't want to affect whole development environment.
  • Extensions should NOT be runner specific. We should have ability to use any set of extensions, when executing TestsA.dll; and any other predefined set of extensions, when executing TestsB.dll.

Based on this, I would like to say that runner/engine should be smart enough to understand what extensions ProjectA or ProjectB want to use.

@CharliePoole
Copy link
Collaborator

I think the first point is overstated. Environment variables may be machine-scoped but may also be user-specific. It depends where they are set and by whom.

The second point is true when the extension point for which the extension is written is part of the engine. But if the extension point is in a runner, then extensions have to be runner-specific. Note that right now, extensions are specific to a particular copy of the engine. So their specificity depends on how engines are located and used. For example, extensions to the copy of the engine that is installed by the VS adapter are runner-specific and (usually) user-specific.

@nvborisenko I'm not arguing for or against a particular approach here. I'm only trying to point out that there are lots of different sides of the problem and that it's probably not a good idea to make absolute rules saying that anything MUST or MUST NOT be done a certain way. We can each only say what type of setup would be helpful to us.

@CharliePoole
Copy link
Collaborator

Moving this to the 4.0 milestone for resolution. While it could probably be done in a non-breaking way, I think being allowed to break existing behavior in a new major release gives us a bit more freedom to rethink the loading of extensions.

@CharliePoole CharliePoole added this to the 4.0 milestone Dec 1, 2021
@CharliePoole
Copy link
Collaborator

This has been sitting for a long time, and I would like to revive it and deal with it. Since the 4.0 release seems not very close with nobody working on it, let's review what has been suggested here as a potential new feature for 3.x. Thoughts?

@CharliePoole CharliePoole self-assigned this Oct 3, 2024
@CharliePoole CharliePoole modified the milestones: 4.0, 3.19.0 Oct 3, 2024
@nvborisenko
Copy link
Contributor Author

I raised it, and I have workaround for it (adding *.addins file at build stage). So user is able to install my package, which will produce correct addins file, which is consumed by nunit runner later (vstest runner).

Another trend I see is executable test assembly - like NUnitLite, but now is officially advertised.

@CharliePoole
Copy link
Collaborator

@nvborisenko How do you see executable test assemblies as related to extensions?

@CharliePoole
Copy link
Collaborator

CharliePoole commented Oct 4, 2024

PROPOSED New Feature... UPDATED 21 Oct 2024

  1. This applies only to engine extensions, the only type we currently have. There is a future possibility for other types of extensions (e.g. to the console runner) which may work differently.
  2. The API used by extensions, including the interfaces they must implement ant the attributes used to annotate them will remain unchanged.
  3. The search algorithm for extensions will remain unchanged EXCEPT that it will allow multiple "root directories."
    3.1 The default initial directory, i.e. the directory containing the engine itself will remain unchanged and will always be used in order to ensure backward compatibility.
    3.2 If the entry assembly is located in a different directory, that directory will be used as a second default start point. UPDATE: It turns out that the tool executable is not used as the Entry Assembly, so we will need to have code that detects the top level of a standalone executable.
    3.3 Additional directories specified by the user will be used to supplement the initial default directories and will be searched in exactly the same way.
    3.4 The existing ability of the extension service to select compatible assemblies and pick the latest version of each will continue to function across the entire set of root directories.
  4. Users will be able to specify directories in which to search for extensions by using the command-line option `--extensionPath:PATH, where PATH is a single directory or a list of directories separated by the system directory separator character.
  5. Directories may also be specified using the value of environment variable NUNIT_EXTENSION_PATH, with the same format as the command-line option.
  6. Examination of the root directories will be in the following order: (1) Initial Directory (2) Directories listed in the environment variable, in order of appearance (3) Directories listed in the command-line option, in order of appearance.
  7. There will be two syntax additions to the .addins file. I will use one or the other of them to try to deal with the problem of standalone executables.
    7.1 A path may start with ^^ to indicate up as many levels as needed to find a match. IOW, it's like ** but in reverse.
    7.2 A path may start with $EXE$/ to represent the entry assembly.

NOTE: The order of examining directories has no effect other than possible differences in efficiency. The latest version of any extension should always be chosen. If users want a particular version, they should ensure that no later version is included in the specified paths.

@nunit and others: Please give me your questions and comments on this, even if you commented earlier in the thread.

@jnm2 Some of this matches some ideas you have put forward in the past.

@CharliePoole
Copy link
Collaborator

Here's a new wrinkle I hadn't seen before but which will have to be dealt with.

Our netcore runner is a standalone executable, executed as a tool. Standalone executables are actually unmanaged assemblies with the various managed assemblies contained in an directory structure internal to the project. For example, the engine inside the NUnit.ConsoleRunner.NetCore package is found inside the package at relative path .store/nunit.consolerunner.netcore/3.18.3/nunit.consolerunner.netcore/3.18.3/tools/net6.0/any/nunit.engine.dll. (NOTE: the duplication in that path is not an error!)

That same directory contains our standard nuget .addins file but it points to nothing, since there are no extensions bundled with the executable. For it to work with "normal" unbundled extensions, the paths in the file would have to begin with ../../../../../../../../ and continue on from there!

This suggests that we may wish to redefine the base path for the addins file as that of the executable itself. For our recent releases, the console exe is in the same directory as the engine, so this would not be a breaking change. (I'll have to research to see how far back this is true.)

Alternatively, this could be handled by adding an additional root directory automatically when dealing with a standalone executable. The engine would, in fact, need to detect that it was part of a standalone executable and find it's way up to the top level. That seems feasible.

@OsirisTerje @veleek @nunit Any other ideas about how to handle this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants