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

Cannot run a project file using --process:Separate #546

Closed
CharliePoole opened this issue Feb 17, 2019 · 10 comments · Fixed by #549
Closed

Cannot run a project file using --process:Separate #546

CharliePoole opened this issue Feb 17, 2019 · 10 comments · Fixed by #549
Assignees
Milestone

Comments

@CharliePoole
Copy link
Member

See TestCentric/testcentric-gui#272.

This can be duplicated in the latest master by using the console runner with the nunit project loader extension installed. Easiest way to do this is to run the Package target from the command-line and then change into the subdirectory of images that was just created. Run a command like...

nunit3-console <path to project> --process:Separate.

In my own tests, I used the GuiTests.nunit project from the test-centric GUI as well as the solution file for the GUI.

It appears that the DirectRunner is eventually invoked to attempt to load the project file itself. This doesn't happen when using no option or with --inprocess, --process:Single or --process:Multiple.

@ChrisMaddock
Copy link
Member

I think this is the same as #439, but I'll close that one as a duplicate, as this has far more information!

@ChrisMaddock
Copy link
Member

@CharliePoole - I brought over pri:low from #439 - which I'd originally used as nobody has ever reported this issue from production use. Is it a higher priority for the GUI? Or did you find it the same way I did, through testing different combinations?

@CharliePoole
Copy link
Member Author

The GUI bug is high priority because it's an option offered to the user that doesn't work. Of course the same could be said for the console... we accept the option but it doesn't work.

However, I agree that it's not an option that gets a lot of use. One thing I could do in the GUI is to disable the option and wait for somebody to complain.

I'm a bit more concerned that it may highlight some fundamental error in how tests are being handled by the DirectRunner, which is the runner that eventually runs every test.

@CharliePoole
Copy link
Member Author

Debugging features that depend upon the installation of an extension can be tricky. What I did is copy the addins directory from an actual installation into the project's bin\Debug directory. That works very well.

Using the --debug-agent option of the console, I was able to determine that the remote runner created by the agent is passed the original top-level package, which is structured like this:

Top-level (null name) package
    * .nunit package
        * assembly package
        * another assembly package
        ...

The DirectRunner, however, is written to assume that it has been given either an assembly or a project containing assemblies - i.e. either one or two levels, rather than three. This assumption was actually true in the V2 console runner and GUI. We had the ability in V2 to open one assembly, multiple assemblies or one project containing multiple assemblies. In NUnit 3, the possibilities have increased but the runner was never updated.

Essentially, in NUnit 3, any runner may have to deal with multiple assemblies distributed in an arbitrary hierarchy. Currently that hierarchy is limited to a miaximum of three levels - command-line, project and assembly - but that could easily change, for example, if some project layout supported nested projects.

That means every runner needs to be able to find all the assemblies in a given hierarchy, run the tests and re-assemble the results in a parallel hierarchy that includes summary nodes for each project. That will call for more change than what is needed for this issue alone, however. For this issue, we only need the direct runner to know how to locate all the assemblies in an arbitrary package hierarchy. I believe that code already exists, since the AggregatingTestRunner already does this. If it's not already centralized in a utility class it probably should be.

@ChrisMaddock
Copy link
Member

@CharliePoole - off the back of this issue, would you mind doing a quick write up of the different test runners, and what each one's intended purpose currently is? I'd thought the AggregatingRunner should be responsible for unpacking assemblies, and the DirectRunner should only ever recieve a single assembly to deal with - but your comments above suggest I'm wrong on this...

(Comment written without code and based on memory...that may not well reflect the current reality of the codebase!)

@CharliePoole
Copy link
Member Author

@ChrisMaddock Sure, I can do that. Do you think it should be a separate writeup or a major comment in each runner file?

@CharliePoole
Copy link
Member Author

In the specific cae of DirectRunner, imagine a command-line like

nunit3-console test1.dll test2.dll test3.dll --inprocess domain:Single

We are saying "run these three test assemblies in process, all in the same AppDomain. The TestDomain runner would create the domain and the base DirectRunner.LoadPackage would be called with a package containing three subpackages. The direct runner would create three drivers, one per assembly, all running in the same AppDomain.

The same thing would happen if we had used --process:Separate.

@ChrisMaddock ChrisMaddock added this to the 3.10 milestone Feb 23, 2019
@CharliePoole
Copy link
Member Author

@ChrisMaddock Pinging you on my earlier question. You asked for a writeup of the different test runners and I wanted to know what form and where you'd like to see it.

@ChrisMaddock
Copy link
Member

Sorry, missed these notifications. I'd originally meant separate, but a comment sounds like a much better idea. And thanks for the direct runner clarification!

@CharliePoole
Copy link
Member Author

I'll do a PR with the comments

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.

2 participants