Duplicate test-run element

[samcragg]

Using the latest package from NuGet (3.4.0-beta-3) I'm getting a test-run element nested inside another test-run element (containing the same data) inside the TestResult.xml file. This is causing problems for AppVeyor to parse it. Looking at the spec (https://github.com/nunit/docs/wiki/Test-Result-XML-Format) there should only be a single test-run element.

If I switch to the 3.4.0-beta-2 package then the TestResult.xml file is as expected and can be parsed by AppVeyor.

I've attached my output and project.json - the output is produced by running the dotnet test command.

DuplicateRootTest.zip

[dougkpowers]

The XML file you attached show a single test-run element. Was the wrong file attached or is it another element that is causing problems?

Nonetheless, the issue may lie with the agent (https://github.com/nunit/nunit.portable.agent). The most recent release of dotnet-test-nunit used an updated version of NUnitLite To isolate the issue, build a local copy of dotnet-test-nunit from (https://github.com/dougkpowers/dotnet-test-nunit). This fork has the latest dotnet-test-nunit changes, but still uses the old agent. If it works, the issue is related to the agent (and you should open your issue within that project). If it still doesn't work, then that would indicate the problem does actually like with dotnet-test-nunit, which would be odd.

[samcragg]

Sorry about the wrong file, it was generated from the beta2 which worked.

I built the fork you mentioned and that produces the correct output so I'll close this issue and try to repro it in the nunit.portable.agent project.

Thanks for your help though in isolating the issue

[ChrisMaddock]

Reopening, as although the portable agent change caused the problem, the fix might be best made here.

Previously the agent could only run one assembly at a time. This was changed for use in the xamarin runners, and to prevent that code needing to be duplicated in multiple places.

The dotnet-test runner (I believe) is still running assemblies one-by-one and amalgamating the results. I thought that would still stand up, however, looking again there are multiple different implementation of the ResultSummary and ResultReporter classes across projects, which may have confused me last time.

The most sensible fix, is probably to have the dotnet-test runner load and run all the assemblies together, and have the portable agent control this. I don't know how easily that will just slot in however. Alternatively, whichever resultsummary/resultreporter are in use here, could control merging test-runs, but the former seems more sensible.

I initially marked this pri:critical, although given the console/VS runner are unaffected, perhaps that was OTT. I'm not sure I'd have the time to put in a fix before next week, if anyone else is able to, please go ahead.

[dougkpowers]

I can confirm that the dotnet-test-runner does run the assemblies one-by-one. In other words, it tests one project at a time. More accurately, the dotnet-test-runner is a slave of the dot net core test harness, which runs tests one project at a time. That also is by design. You can actually have different test runners for each project, which makes test configuration (and running) project specific rather than solution specific. The dotnet-test command doesn't even have an option for running tests for more than one project at a time (https://docs.microsoft.com/en-us/dotnet/articles/core/tools/dotnet-test)

As a result, I do not believe it is feasible to have the dotnet-test runner load and run all the assemblies together. Therefore, whenever there is another tool running tests across multiple projects, it will have to be responsible for merging the results.

[ChrisMaddock]

@dougkpowers - if that's the case, then sounds like I'm barking up the wrong tree. 😄 I'm not familiar with dotnet tooling, just looking at the code in this repo.

https://github.com/nunit/dotnet-test-nunit/blob/master/src/dotnet-test-nunit/TestRunner.cs#L151

I was looking at the above method, which loops through _options.InputFiles and loads/runs each one. If, as I believe your suggesting, _options.InputFiles can only ever contain a single file, then what I was assuming is irrelevant.

[dougkpowers]

@ChrisMaddock -- Ah yes, if you run the dotnet-test-nunit command directly then you can test multiple assemblies at once. I was assuming that the dotnet-test-nunit runner would actually be invoked via the dotnet-test command, which in turn, runs dotnet-test-nunit. In this scenario, I believe it would only test one project at at time. Of course, I suppose you could have one test project with more than one test assembly (head starting to hurt).

So there are several scenarios to consider: 1- Two projects (one assembly each) with tests being invoked via dotnet-test 2- Two projects (one assembly each) with tests being invoked directly via dotnet-test-nunit 3- One project (multiple assemblies...suppose this is possible) with tests being invoked via dotnet-test (I suspect this may be a way to get dotnet-test to run tests across multiple assemblies in one run)

[ChrisMaddock]

Have verified this - it also happens on every run, and isn't limited to cases with multiple assemblies.

Unfortunately it's possibly a little nontrivial - as the TestListeners in dotnet-test-nunit have the assembly path passed in, so setting up a single testlistener for everything may not work out. :-/ Meaning the better option may be to not use the ResultSummary as is done, in either agent, or dotnet-test-nunit.

Not sure yet, will try and have a look this weekend. Any thoughts, @rprouse?

[ChrisMaddock]

Ok, I've dug further.

In an ideal world, the agent would take care of running multiple assemblies, however, this isn't possible with dotnet-test-nunit as each assembly requires it's own testlistener, for the line-number information etc. So instead, we'll need to keep running the input files individually in this runner, and the issue is just summarising the results accurately.

It's actually simpler than I first thought - there's just a bug in ResultSummary.cs in the agent, which doesn't correctly summarise multiple test-run elements. PR incoming for that, we'll then need to update this project to use a new agent.

[CharliePoole]

Rather than trying to handle multiple test-run elements, I think you need to question why you have them in the first place. Consider that the standard nunit runners never have to deal with this situation. What's different in this runner that causes the situation to arise in the first place?

[ChrisMaddock]

The latest agent now returns a full test-run rather than a test-suite - this allows it to run multiple assemblies, and prevents that functionality needing to be implemented in multiple runners.

Unfortunately dotnet-test-nunit currently needs to integrate on a per-assembly level, to provide correct code-navigation data. Therefore it still needs to run the assemblies one-by-one, and amalgamate the test-run nodes it now receives from the agent, rather than the test-suites it previously received.

The main point that persuaded me this was the right course of action, was that ResultSummary.cs already has partial functionality to amalgamate test-run's - it just had a bug. (See https://github.com/nunit/nunit.portable.agent/pull/16) I suppose the other way-around this would be to revert the agent changes to allow multiple assemblies, and instead re-implement the multiple-assembly code in the nunit.xamarin project, however, it does seem cleaner to me if the agent can take care of that.

[ChrisMaddock]

I guess in many ways this is a discussion as to the role of the agent. Is it going to be a simple, version-agnostic pipe to the framework, or is it going to provide more functionality required in all runners, and work as a lightweight-'engine'?

I'd favour the latter - reduce the code we duplicate, and provide a simple clean interface to runners.

[CharliePoole]

If it's a lightweight engine then life will be simpler I think if it works similarly to the engine. The engine returns multiple assembly results without use of a wrapping top-level element. Only the master test runner applies that final wrapper.

Is it the case that the engine was originally created as a simple wrapper that you're now trying to make into a more complete engine?

I'm interested in this because I'm hoping we'll someday merge all this back into the engine.

[ChrisMaddock]

Sorry @CharliePoole - had to go back to work earlier! Will answer this with your other comment over on https://github.com/nunit/nunit.portable.agent/pull/16 - save us having the conversation over two different places!

[johnkors]

@samcragg , you sure it works w/ beta2? I get nothing out of AppVeyor for beta2.

https://ci.appveyor.com/project/JohnKorsnes/nocommonsnet/build/1.0.25

[samcragg]

@johnkors In my project.json I'm using "dotnet-test-nunit": "3.4.0-beta-2", and it works fine on my build, however, I am manually uploading the test results (detailed here), as the build didn't pick them up automatically. So in my build script I have this (in PowerShell) after running the tests:

$wc = New-Object 'System.Net.WebClient'
$wc.UploadFile("https://ci.appveyor.com/api/testresults/nunit3/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\TestResult.xml))

Here's the output: https://ci.appveyor.com/project/samcragg/crest/branch/master/tests

The problem I had with beta3 was even manually uploading them didn't work, as there was an extra test-run element so their parser couldn't handle the input.

Hope it helps

[johnkors]

Akward to talk to Appveyor - in a Appveyor build at Appveyor - thru their public API instead of it just.. working oob :) Ah well. I'll wait this one out, I think :)

Thx for the fix, though! :)