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

nUnit project loader does not work when --inprocess is set #386

Closed
SoeGoe opened this issue Mar 22, 2018 · 9 comments
Closed

nUnit project loader does not work when --inprocess is set #386

SoeGoe opened this issue Mar 22, 2018 · 9 comments
Assignees
Milestone

Comments

@SoeGoe
Copy link

SoeGoe commented Mar 22, 2018

When starting nunit3-console 3.8.0 with an nUnit project (requiring nUnit project loader) and either the --inprocess or --process=Single parameter, I get an "File type is not supported" error message. If I use exactly the same parameters but leave out the --inprocess, the project file is recognized correctly.

(Tested with nunit3-console msi package on Windows 10.)

This seems to be a regression in 3.8.0; 3.7.0 worked fine.

@ChrisMaddock
Copy link
Member

ChrisMaddock commented Jun 5, 2018

Marking confirmed given two people have now encountered this issue.

@ChrisMaddock
Copy link
Member

I had a quick dig. H*ck this stuff get's confusing! 😅

Looks like this issue was caused by the fix using multiple agents in the default case when reading NUnit projects in ee01734

The removal of the 'shim' to make packages containing single projects promoted to have the project as the top-level item is what will have exposed this project. This issue is then with how the runner is chosen. We currently have this:

            switch (processModel)
            {
                default:
                case ProcessModel.Default:
                    if (projectCount > 0)
                        return new AggregatingTestRunner(ServiceContext, package);
                    else if (package.SubPackages.Count > 1)
                        return new MultipleTestProcessRunner(this.ServiceContext, package);
                    else
                        return new ProcessRunner(this.ServiceContext, package);

                case ProcessModel.Multiple:
                    return new MultipleTestProcessRunner(this.ServiceContext, package);

                case ProcessModel.Separate:
                    return new ProcessRunner(this.ServiceContext, package);

                case ProcessModel.InProcess:
                    return base.MakeTestRunner(package);
            }

It looks to me like the newly added:

    if (projectCount > 0)
       return new AggregatingTestRunner(ServiceContext, package);

Should also be hit for all other process models - i.e. a package which contains solely a project should always return an AggregatingTestRunner - which defers the decision as to which TestRunners to use till after the point a project has been expanded.

Regarding respecting the command-line-specified process model over the project-process-model, hopefully the ProjectService should take care of passing that down to subpackages, and invalid combinations should be flagged in the normal way.

Anyone have any thoughts, either way?

@rprouse
Copy link
Member

rprouse commented Jun 6, 2018

According to the comment above, it would appear that it is not that line, but the fact that AggregatingTestRunner is not respecting the individual file's settings.

// If we have multiple projects or a project plus assemblies
// then defer to the AggregatingTestRunner, which will make
// the decision on a file by file basis so that each project
// runs with its own settings. Note that bad extensions are
// ignored rather than assumed to be projects. This doesn't
// really matter since they will result in an error anyway.
if (projectCount > 1 || projectCount > 0 && assemblyCount > 0)
    return new AggregatingTestRunner(ServiceContext, package);

It should work though because AggregatingTestRunner goes through each child TestPackage and creates a child test runner for each by calling back into DefaultTestRunnerFactory. At that point, the project should be expanded to assemblies and respect the process model.

public IList<ITestEngineRunner> Runners
{
    get
    {
        if (_runners == null)
        {
            _runners = new List<ITestEngineRunner>();
            foreach (var subPackage in TestPackage.SubPackages)
            {
                _runners.Add(CreateRunner(subPackage));
            }
        }

        return _runners;
    }
}

protected virtual ITestEngineRunner CreateRunner(TestPackage package)
{
    return TestRunnerFactory.MakeTestRunner(package);
}

Looking at the code, my guess is that we do not copy the process model to child packages when we expand the project or there is something else wrong with the child packages that are created?

@ChrisMaddock
Copy link
Member

Rob - I think you might be mis-reading that condition? Or I'm misunderstanding you! 😅

That line to create an AggregatingTestRunner is only hit when there are multiple projects, or projects plus assemblies in the top level package. This case is a single project, which by-passes that line, and falls into the next section.

I do think that line should be hit for this case, however!

@rprouse
Copy link
Member

rprouse commented Jun 6, 2018

@ChrisMaddock you are right, I was misreading your statement and only looking at the case ProcessModel.Default statements. I think we need the same if statement in the case ProcessModel.InProcess:, but not the others.

@CharliePoole
Copy link
Contributor

As originally written, a single project was expected to use a process runner by default. The agent would then look at the DomainUsage setting to decide whether to run in a single or multiple domains. In process, we should fall through to examining domain usage.

@anivala
Copy link

anivala commented Sep 30, 2018

Please also check that it runs correctly with the command line parameter --process=Separate. I was trying to ensure that the tests of different assemblies would not run in parallel and added --process=Separate to a command line for nunit3-console.exe 3.9.0 which was previously working. But after adding --process=Separate I get that "File type is not supported". I'm trying to work around it now by adding --agents=1 instead, which doesn't seem to cause the "File type is not supported" error.

@ChrisMaddock
Copy link
Member

@anivala this is tracked as #439

@ChrisMaddock
Copy link
Member

This is now fixed in master, possibly by #622. 😄

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