nunit / nunit-console

NUnit Console runner and test engine
MIT License
214 stars 152 forks source link

Explore command (--explore) does not report issues while generating Test Cases #658

Open DominikJaniec opened 5 years ago

DominikJaniec commented 5 years ago

I found strange inconsistency in behaviour of NUnit.ConsoleRunner between explore and execute when there are "problematic" test cases - i.e. an exception thrown when resolving TestCaseSource values. Generally speaking, I would like to:

  1. Get some indicator that exploration (i.e. result of --explore) didn't went quite well, as there were issues with generating cases.
  2. See more details, when given test could not executed (by its "exact name") because cases were not generated.

If current behaviour is "by design", I would be very glad to get an explanation why there is such difference. Also, I would like to find such information in documentation, but I was unable to.

I'm also very surprised that NUnit will not complain much, when it was instructed to execute test (by "exact name"), but such test could not be found (e.g. nobody yet created one). It looks risky (at least for me), to return 0 when no test (especially requested) was executed, as it will hide issues with infrastructure which uses NUnit (e.g. incorrectly provided tests names). On the other hand, it is not my main concern here.


Nonetheless, I've prepared demo (NUnitInconsistency.zip, sources), and I believe that my explanation isn't convoluted much. All outputs are trimmed to significant ends.

It is a simple .NET (4.7.1, class-lib) project with NuGet dependencies to NUnit (3.12.0) and NUnit.ConsoleRunner (3.10.0). It is basically single C# file, which is relevant for this issue:

using System;
using NUnit.Framework;

namespace NUnitInconsistency
{
    public class ExecuteVsExploreTests
    {
        private const bool ShouldCauseProblems = false;

        public static string[] ProblematicSource()
        {
            if (ShouldCauseProblems)
                throw new Exception("Problem!");

            return new[] { "First", "Omega" };
        }

        [TestCaseSource(nameof(ProblematicSource))]
        public void InconsistencyTest(string value)
            => Assert.IsNotEmpty(value, "Just for show");
    }
}

(I) When it's compiled (after NuGet restore, then Debug build), tests could be executed and given assembly can be "tested" via console (cmd.exe):

> packages\NUnit.ConsoleRunner.3.10.0\tools\nunit3-console.exe NUnitInconsistency\bin\Debug\NUnitInconsistency.dll
Test Run Summary
  Overall result: Passed
  Test Count: 2, Passed: 2, Failed: 0, Warnings: 0, Inconclusive: 0, Skipped: 0

> echo Exit code: %errorlevel%
Exit code: 0

(II) Also tests could be "explored" to find their "exact names" in given compiled assembly:

> packages\NUnit.ConsoleRunner.3.10.0\tools\nunit3-console.exe NUnitInconsistency\bin\Debug\NUnitInconsistency.dll --explore
NUnitInconsistency.ExecuteVsExploreTests.InconsistencyTest("First")
NUnitInconsistency.ExecuteVsExploreTests.InconsistencyTest("Omega")

> echo Exit code: %errorlevel%
Exit code: 0

(III) Everything works fine and as expected. However, let's assume that we encounter some problems while generating cases (just change ShouldCauseProblems to true and rebuild). So as a result, when executing tests, we will get appropriate ExitCode, and expected Overall result: Failed with detailed exception & stack-trace (omitted here):

> packages\NUnit.ConsoleRunner.3.10.0\tools\nunit3-console.exe NUnitInconsistency\bin\Debug\NUnitInconsistency.dll
Test Run Summary
  Overall result: Failed
  Test Count: 1, Passed: 0, Failed: 1, Warnings: 0, Inconclusive: 0, Skipped: 0
    Failed Tests - Failures: 0, Errors: 0, Invalid: 1

> echo Exit code: %errorlevel%
Exit code: 1

(IV) Nonetheless, under those circumstance (problem with generating test cases), when someone would like to find what tests are available in given assembly, they will get "everything" without problems, but at the same time, tests will be generated without arguments in names (i.e. just name of test), and nobody know that there is an issue:

> packages\NUnit.ConsoleRunner.3.10.0\tools\nunit3-console.exe NUnitInconsistency\bin\Debug\NUnitInconsistency.dll --explore
NUnitInconsistency.ExecuteVsExploreTests.InconsistencyTest

> echo Exit code: %errorlevel%
Exit code: 0

(V) I guess, it is not a big deal, as one can always execute all test under its common name (i.e. without any arguments), and when there is a problem with generating cases, you will find expected behaviour - very similar to (III):

> packages\NUnit.ConsoleRunner.3.10.0\tools\nunit3-console.exe NUnitInconsistency\bin\Debug\NUnitInconsistency.dll --test NUnitInconsistency.ExecuteVsExploreTests.InconsistencyTest
Test Run Summary
  Overall result: Failed
  Test Count: 1, Passed: 0, Failed: 1, Warnings: 0, Inconclusive: 0, Skipped: 0
    Failed Tests - Failures: 0, Errors: 0, Invalid: 1

> echo Exit code: %errorlevel%
Exit code: 1

(VI) In like manner, let's assume that one has got full names of tests with arguments, and only later, the issue with generating test cases come up, but nobody knows that yet. Then, one tries to execute single test by its "exact name":

> packages\NUnit.ConsoleRunner.3.10.0\tools\nunit3-console.exe NUnitInconsistency\bin\Debug\NUnitInconsistency.dll --test "NUnitInconsistency.ExecuteVsExploreTests.InconsistencyTest(\"Omega\")"
Test Run Summary
  Overall result: Passed
  Test Count: 0, Passed: 0, Failed: 0, Warnings: 0, Inconclusive: 0, Skipped: 0

> echo Exit code: %errorlevel%
Exit code: 0

Clearly "no tests executed", but also no errors printed - with information that such test case could not be found, as no cases could be generated. Moreover, let's assume that the one is a script (CI/CD agent) and doesn't care about output, it only care about exit code, but in such case, everything went OK, as %errorleverl% is 0.

CharliePoole commented 5 years ago

IIRC we once had a specific error for this situation (filter specified with no matches found) in NUnit V2. I suspect it got lost because it was unclear where the responsibility for an error should lie.

I think the runner is the right place. If the framework is called with a filter that gives no matches, it should simply return no matches. The framework doesn't know why it was called. Maybe it's being called on multiple test assemblies, for example, where only one of them will have the match.

Similarly, I don't think the engine should give an error for exactly the same reason.

That leaves the console runner itself. It knows that the user expected to find some specific tests with one or more assemblies or projects. If none are found, it can point that out.

However, in the case of explore or run, it can only do that if no tests are found. If some are found, but not all those you hoped for, it doesn't seem as if we can do anything more. The invalid suite is displayed on a run or in the XML output of explore. The default explore lists only test cases, so of course it won't show test cases that were never created.

In retrospect, --explore's default output may not have been the best choice. :disappointed:

ChrisMaddock commented 5 years ago

I believe issue 1 is a duplicate of https://github.com/nunit/nunit/issues/1381 (Although an excellent bug report for it - much appreciated!) This one, I agree is a clear cut bug which should be fixed. I'm not totally sure off the top of my head if the code to do this lies in the framework or engine. (I think perhaps framework?)

Issue 2: my expectation is that this console method would still load the entire assembly first, and I would expect this to fail at that point. (At least up until we implement https://github.com/nunit/nunit-console/issues/384!) The exception must be being swallowed somewhere, and I would argue that if you fail to build all test cases, the only 'known' you have is that you can't return a full response to the test filter, so the test run should fail.

@DominikJaniec - We'd appreciate pull requests for these issues, if you'd be interested? (But I would wait for some more members of @nunit/engine-team to comment first - as Charlie and I perhaps disagree on the desired behaviour!)

ChrisMaddock commented 5 years ago

Just to be completely clear, I would propose the following:

Charlie - what do you think? 🙂

DominikJaniec commented 5 years ago

@ChrisMaddock thank you and yes, my main issue is sort of duplicate of #1381, however both ConsoleRunner and VS-Adapter behave similarly and in the same time differently then described at: https://github.com/nunit/nunit3-vs-adapter/issues/144

I fully agree with your first bullet. However, I think I like idea of selective loading of tests and also current behaviour: execution of all requested tests, even thou generation of cases failed for some but other (not requested) tests.

And about my possible contribution, I'm not sure if I'll manage, but I may take a look, when everything will be settled and it won't require to turn everything out.