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

Missing stack trace when exception occurs during OneTimeSetUp #2466

Closed
OsirisTerje opened this issue Oct 2, 2017 · 27 comments · Fixed by #4469
Closed

Missing stack trace when exception occurs during OneTimeSetUp #2466

OsirisTerje opened this issue Oct 2, 2017 · 27 comments · Fixed by #4469

Comments

@OsirisTerje
Copy link
Member

@ericnewton76 commented on Sat Dec 17 2016

image

Missing the stack trace for the error if it bubbles up from a OneTimeSetUp method.


@ericnewton76 commented on Sat Dec 17 2016

I have forked vs-adapter and I'm looking through the code, and this type of scenario isn't tested.

https://github.com/ericnewton76/nunit3-vs-adapter/commit/09901cd4d26afbb4aa6dc02f28ed79c5abb9556c
I added a class to mock-assembly project (at the end of MockAssembly.cs) but i'm not sure how to run the tests against this new mock assembly.

Additionally I wonder if nunit itself has the stack trace for the exception during OneTimeSetUp and if the vs-adapter is displaying all that it has.


@ericnewton76 commented on Sat Dec 17 2016

Ok, attempted to run build script, because I see it includes test running, but MSBuild giving me some kind of VS2017 sxs installation problem, so dont install VS2017 yet. The VS developers are lamenting the fact that VS2015 cant build VS2017 at the moment. I'm guessing this is related to my problem. with getting msbuild to build successfully. github/VisualStudio#655 (comment)


@OsirisTerje commented on Sun Dec 18 2016

I can repro this, and I see we don't add the OneTimeSetup crash information to the output. The information comes from the TestFixture node, and the site info is "Child".
@CharliePoole @rprouse : I see the Setup and Teardown have its own site names, but OneTimeSetup and OneTimeTearDown are not listed there. And I can't see any of these site names in the output result file. I only see Child and Parent.


@CharliePoole commented on Sun Dec 18 2016

One time setup is the parent setup. It's difficult to know what to do here since VS has no place to report suite results but only test cases. We could report the suite result on each test case but that's potentially a lot of duplication. I compromised by reporting only the message but the stack trace could be reported as well.


@ericnewton76 commented on Sun Dec 18 2016

How does nunit handle an exception in onetimesetup? Does it basically mark the whole test fixture as invalid, or does it keep retrying onetimesetup for each test in the fixture (Which to me would makes most sense)


@rprouse commented on Sun Dec 18 2016

It marks the whole fixture as failed, it doesn't retry. We assume that your setup code is deterministic and will fail consistently.


@OsirisTerje commented on Sun Dec 25 2016

Is the output I see in the console output (testresult.xml) identical to what the adapter gets as a response?


@CharliePoole commented on Mon Dec 26 2016

Yes, the adapter gets the entire XML result in pieces as each test completes. (It could also get the completed result but it doesn't need it since the info is given to VS as it arrives.) The result of a test suite is pretty much ignored, since it's not associated with any of the tests that have been discovered by VS. You can see that happen here: https://github.com/nunit/nunit3-vs-adapter/blob/master/src/NUnitTestAdapter/NUnitEventListener.cs#L148

The result of a test case is handled in the method right above that one. It records what is contained in the test case result.


@OsirisTerje commented on Mon Dec 26 2016

Thanks! And what is the real meaning of the term/attribute "site" ? It can take values like Child and Parent, but also SetUp and TearDown and the like ?


@CharliePoole commented on Mon Dec 26 2016

In the framework, it's an enum. We use the enum names as attribute values. The default is "Test" which is what usually happens. The code in the adapter only logs a message for setup and teardown suite failures, since those don't get reflected anywhere else.

Note that the one time setup error message was being shown in the original issue description. That indicates it must be coming in the test case failure. You want to examine whether that's happening with the stack trace too. I suspect not.


@CharliePoole commented on Mon Dec 26 2016

BTW these attributes should all be documented here: https://github.com/nunit/docs/wiki/Test-Result-XML-Format


@carlin-q-scott commented on Wed Apr 19 2017

So if I want a OneTimeSetup that gives me sufficient detail to figure out the cause of the failure, should I do something like this?

static bool oneTimeSetupRan;

[SetUp]
public void OneTimeSetup()
{
    if (!oneTimeSetupRan)
    {
         throw new NotImplementedException();
         oneTimeSetupRan = true;
    }
}

@CharliePoole commented on Wed Apr 19 2017

@carlin-q-scott That would definitely simulate the one-time setup for a fixture. It's a pity to have to go to that trouble though.


@CharliePoole commented on Wed Apr 19 2017

@nunit/vs-extensions-team What do we want to do here? Is it simply a limitation of the VS interface? Should we try to report the stack trace for each error? That would require our remembering the error info for each one time setup failure and reporting it for the corresponding test cases. Non-trivial, especially in the presence of multiple fixtures running in parallel.


@blorger commented on Fri Aug 18 2017

This is not just limitation of VS interface. NUnit simply does not provide call stack information. Problem remains if you execute tests with NUnit console, no call stack on console output or in result file.
Why not simply change content of a message. Provide entire exception description rather than just exception message. This way there will be no need to debug a test or modify it (move one time setup code inside test, or wrap each setup method in try/catch block to log exception ...) just to get information where failure occurs.


@OsirisTerje commented on Sun Oct 01 2017

If I have an exception in the OneTimeSetup, for each test it is reported, like this:
OneTimeSetUp: ClassLibrary11.WhateverException : Exception of type 'ClassLibrary11.WhateverException' was thrown.

The stack trace is coming with the suite finished event a bit later, but then the test results have already been sent.

I think it would be better just to return the full exception stacktrace with each failing tests. It would only slow down failing tests, so that should be no big matter. And then no changed code is needed for the adapter.

@rprouse / @CharliePoole If you agree, can we add an issue on NUnit for this, or move this to NUnit ?


@CharliePoole commented on Sun Oct 01 2017

If we want to make it look a certain way for VS, then I think we should make it look that way in the adapter. NUnit itself attaches the error to the test that failed, which was the fixture. That's a pretty fundamental thing about how NUnit looks at tests: everything is a test - test cases, methods, classes, namespaces and the assembly. VS looks at the tests as a list of cases. Since the VS view is simpler, it should be possible to make things appear that way if it's really desired.


@OsirisTerje commented on Sun Oct 01 2017

Yes, we can do this in the adapter, but NUnit is reporting this per test case, just with less information. It has split the information of the exception between the test case and the fixture, with the test case reporting the exception and the message, and the fixture reporting the same plus the stacktrace.


@CharliePoole commented on Mon Oct 02 2017

Add the issue to NUnit in that case, so we can discuss it there. I think it should be a new issue, since up to now this has been about how the adapter displays what it gets from NUnit, rather than changing what NUnit delivers.

We have a similar problem in the console runner, because the console runner historically only counted test cases and only displayed test case failures and errors. Over time, we started displaying more info. But the console has access to the entire XML, so modifying the output was easier.

@CharliePoole
Copy link
Contributor

@OsirisTerje You moved the issue rather than adding one, which is OK, although I suspect we will end up having to do something in the adapter as well. However, the issue has no info in it about the framework, what it's doing wrong, how it should do things differently. How are you proposing the framework should be changed?

@jnm2
Copy link
Contributor

jnm2 commented Feb 18, 2018

Yes, we can do this in the adapter, but NUnit is reporting this per test case, just with less information. It has split the information of the exception between the test case and the fixture, with the test case reporting the exception and the message, and the fixture reporting the same plus the stacktrace.

I played with the code a bit. The current behavior makes the most sense to me. I think OneTimeSetUp and OneTimeTearDown failures should only report information at the fixture level. There's no natural association from the once-per-fixture failures to any particular child test.

If OneTimeSetUp fails, of course the child tests should fail because they cannot run.
If OneTimeTearDown fails, it's too late to fail the child tests because their results are finalized before the fixture is torn down.

Why don't we leave NUnit pure in this regard and do any shimming in the adapter like Charlie mentioned where necessary to translate the richer NUnit results to VSTest results?

@rprouse
Copy link
Member

rprouse commented Feb 19, 2018

With the new hierarchical test window in the latest version of Visual Studio, will this become a non-issue as the error will be reported at the fixture level?

@jnm2
Copy link
Contributor

jnm2 commented Feb 20, 2018

With the new hierarchical test window in the latest version of Visual Studio, will this become a non-issue as the error will be reported at the fixture level?

I can confirm that the error is not reported at the fixture node level. The parent nodes are just group headers at present:

image

(Yes, the icon is distorted. Yes, it's finally irritated me enough. https://developercommunity.visualstudio.com/content/problem/200622/test-explorer-icons-are-distorted-in-hierarchy-mod.html)

@jnm2
Copy link
Contributor

jnm2 commented Feb 20, 2018

So what should the adapter do?

For each test case, if the NUnit result site is Parent (for any kind of result), the adapter could go up the chain until the site is no longer Parent and convert that result to use as the test case's VSTest result. @OsirisTerje what do you think of that?

@tobyash86
Copy link

I am experiencing the same behavior. Is there any progress regarding this case?
Sometimes when fixture setup fails I need to debug it to find out where it has failed.

@jnm2
Copy link
Contributor

jnm2 commented May 28, 2018

There has been no progress on this beyond what you see.
If I understand this properly, either NUnit needs to start giving full stack trace information to child sites as well as the actual site, or else the adapter needs to compensate for the fact that it can't report parent nodes to VSTest and do the merge itself.

@CharliePoole
Copy link
Contributor

Several of us suggested that this is not an issue for the framework at all, but the adapter. Should we perhaps decide and close this particular issue if we are not going to do anything on it?

@jnm2
Copy link
Contributor

jnm2 commented May 29, 2018

I do think that unless the adapter is blocked, we should close this one. I might not have the full picture so I wanted to wait for @rprouse or @OsirisTerje to confirm.

@jnm2
Copy link
Contributor

jnm2 commented Jun 28, 2018

⚠ @YuriyOborozhnyi brought this to my attention while working on #2906. This test run passes if you use the adapter:

public static class SomeFixture
{
    [Test]
    public static void SomeTest()
    {
    }

    [OneTimeTearDown]
    public static void SomeTearDown()
    {
        throw new System.Exception();
    }
}

This seems critically broken to me. NUnit 3.10.1, adapter 3.10.

Repeating #2466 (comment):

There's no natural association from the once-per-fixture failures to any particular child test.

Why don't we leave NUnit pure in this regard and do any shimming in the adapter like Charlie mentioned where necessary to translate the richer NUnit results to VSTest results?

@OsirisTerje My thinking right now is that if there is any fixture-level teardown, the adapter should not report any test results for tests in that fixture until the teardown happens. If there is an error in the teardown, all tests should error. What do you think? Can I open the issue back up in the adapter repo?

@OsirisTerje
Copy link
Member Author

@jnm2 Please open the issue. (Sorry about noticing very late here)

@CharliePoole
Copy link
Contributor

Hmmm... The adapter doesn't have any way to know if there is fixture level teardown without duplicating the analysis done by the framework.

Of course, in one sense, there is always fixture-level teardown, but not always user-speciffied teardown.

What about higher-level teardown? Let's say a teardown in a setup fixture fails. What would we do then? Taken to the extreme, we could say you have to wait till all nested suites are complete, right up to the assembly itself. But then that makes it a batch system, with no progress reported.

So, for all the above reasons, I think you have to do two things in the adapter:

  1. Report exactly what nunit reports to you.
  2. Additionally report any teardown errors.

@zmhh
Copy link

zmhh commented Mar 12, 2021

+1

@jamers99
Copy link

jamers99 commented Mar 30, 2021

Any progress on this? This is very difficult to debug if there is a test failure in the set up or tear down. The stack trace is critical to debugging issues, especially in more complex scenarios.

@CharliePoole
Copy link
Contributor

CharliePoole commented Mar 30, 2021

Since this is only a problem in the adapter, it seems odd that this issue remains in the framework project. If @OsirisTerje were to ask for something specific, then it would make sense. Otherwise, it's just an adapter issue. (I only say this because issues in the wrong place almost never make progress!)

@svengeance
Copy link

@CharliePoole I'm waiting for a response on the adapter on what the best way forward is - I've done a bit of work in it and I'd be comfortable making the change.

I'm concerned that the full exception isn't bubbled up from the Runner, though. I haven't fully delved into it, but it - might - need a change in more than one place.

@CharliePoole
Copy link
Contributor

The framework definitely provides the stack trace, but possibly not in a way that's convenient for the adapter. That's why I thought @OsirisTerje might be the best person to request a framework change.

In my own use of the framework for the GUI runner, I'm able to handle this situation because the fixture result, containing the stack trace, is visible to the user. So I think there are two questions:

  1. How may a runner, which doesn't normally show fixtures as separate entities, display such an error?
  2. Should such a runner have to do the whole job or should the framework somehow assist?

@OsirisTerje
Copy link
Member Author

OsirisTerje commented May 29, 2021

Please check the enclosed adapter. It will output the stack trace for the exceptions in the overall message part. . It will be included in the trx file, and in Visual Studio Console Output window for testing. This is the "easy" way of solving this. It will not come out per test as the tests don't have that information from NUnit, and the work to retrieve that is not trivial. But, with this change now, it will come out in the console output. If needed, some more information can be included.
NUnit3TestAdapter.4.0.0-dev.751.zip

PS: About the hierarchy in the Test Explorer, that is still just fake as @jnm2 mentioned above, and there are no actual nodes we can attach any information to.

@vladdex
Copy link

vladdex commented Sep 13, 2023

@OsirisTerje is there any resolution for this ? I've tried every solution but I cannot get Nunit to report exceptions that appear in OneTimeSetup methods.
Currently running NUnit3Test adapter 4.5.0

@OsirisTerje
Copy link
Member Author

OsirisTerje commented Sep 13, 2023

The stacktrace does flow up from the test:
image
Also from the command line:
image

If you have some specific case, please upload a repro for that, or modify the issue code, which you can find here: https://github.com/nunit/nunit.issues/tree/main/Issue2466

@manfred-brands
Copy link
Member

@OsirisTerje The stack trace is only visible if going to the Output tab and select Tests output. Not in the Test Explorer. There only the exception is shown. The stack trace doesn't seem to be propagated to the test result:

image

I tried to check this in the OneTimeSetUpAndTearDownTests, but the StackTrace is set in the fixure.
However the children have no StackTrace.
It gets lost in CompositeWorkItem

Is this intended or would you like for me to create a PR to pass the StackTrace to the SkipChildren and SetChildWorkItemSkippedResult methods?

@vladdex
Copy link

vladdex commented Sep 14, 2023

@OsirisTerje @manfred-brands
I am seeing this issue when running the tests in an Azure Pipeline, sorry I forgot to mention that in my initial post.
This is how the output looks like
image
image

@OsirisTerje
Copy link
Member Author

@manfred-brands I am not sure what is intended or not here, but I can't see any reason to not propagate it further, so yes, please do that :-) We can then get it out in the adapter to the tests.

@vladdex true, the Azure Pipeline hides the console output messages. I do believe you can get it out using the dotnet test --logger "console;verbosity=normal (IIRC). But if you use the built-in tasks i AP I don't think you can, but changing that in the yaml file should be easy (and if you use the classic pipeline setup, you should move over to yaml based). However, if @manfred-brands can do the PR he suggests, we may get the stacktrace moved to the [first] tests.

@vladdex
Copy link

vladdex commented Sep 14, 2023

@OsirisTerje I tried your suggestion, I'm using the DotNetCoreCLI@2 task in the pipeline.
I get the stack in the console output of the tests, at the end of all test methods in the class, somewhere in the middle of the logs
The stacktrace is still not present in the test results viewer from devops.
image
image

@OsirisTerje
Copy link
Member Author

OsirisTerje commented Sep 14, 2023

@vladdex That's good, as a workaround so far! (I appreciate the screenshot, then we have doc for that situation too)

When @manfred-brands get the PR in, and a subsequent change in the adapter, we can get it out in the test results. It will be a bit kind of fake, since the Test are actually not run at all, but from a users point of view it makes sense, so we'll do it.
It will then be in 4.0 of NUnit and 4.6 of the adapter.

@OsirisTerje OsirisTerje added this to the 4.0 milestone Sep 14, 2023
@manfred-brands
Copy link
Member

I actually made the PR on v13.3. We were thinking of making a 13.4 before 4.0.
Will create one for 4.0 now.

@manfred-brands
Copy link
Member

I have tested this with the Package artifact created from the build: https://github.com/nunit/nunit/actions/runs/6185437669

It now shows the stack trace at the test:
image

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.