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

[NUnit3] Make FixedNameTestMethodBuilder the default strategy #865

Merged
merged 2 commits into from Oct 13, 2017

Conversation

zvirja
Copy link
Member

@zvirja zvirja commented Oct 6, 2017

Closes #862.

In this PR I make the FixedNameTestMethodBuilder strategy (recently introduced in #709) the default one. The reason behind that is to support the VS and NCrunch test runners out-of-the-box if user uses AutoData and InlineAutoData attributes and NUnit3.

The cons is that this strategy makes it a bit harder to troubleshoot if particular test failed:
Test name before: SomeTest(42,"autogeneratedstring")
Test name after: SomeTest(auto<Int32>,auto<String>)

Now we show the placeholders instead of actual values. If the particular test fails, you cannot look on the test input to understand why that happened. Rather, you need to rely on the assertion methods output, which not actually so bad..

I think that being able to work with all the runners by default is more important, than potentially decreased diagnostic opportunities. If user doesn't like how it works now, he could switch the name strategy back by following the described approach.

@moodmosaic @adamchester Please let me know what you think about that.

@zvirja zvirja added this to the 4.0 milestone Oct 6, 2017
@frblondin
Copy link
Contributor

If you plan to change this default behavior in a major release, you could also change the NUnit prerequisite to 3.5+ and remove this hack as it is no longer required with recent versions of NUnit3.

@zvirja
Copy link
Member Author

zvirja commented Oct 6, 2017

@frblondin No, I'd vote to keep that as is. Even if that reflection usage is ugly, for me that is not strong enough reason to upgrade the dependency version. We might have clients using the olders version of NUnit due to some restrictions/dependencies and they will be abandoned 😟

@frblondin
Copy link
Contributor

Fair enough, @zvirja

@Kralizek
Copy link
Contributor

Kralizek commented Oct 8, 2017

Is this baked in into a nightly build? I'm using the latest, 4.0.0-alpha.253, and I don't get the test names to be fixed and therefore discoverable by VS 2017 Live Tests

@zvirja
Copy link
Member Author

zvirja commented Oct 8, 2017

This PR wasn't merged yet, so no. Will be available in 5 days, after the PR is merged.

While this strategy decreased that diagnostics aspect of tests as
now you don't see the actual test input, it allows
AutoFixture.NUnit3 to work out-of-the-box for VS and NCrunch.

If some customers prefer diagnostics information, they are free to
change the default strategy in their solution.
@zvirja zvirja force-pushed the make-fixed-name-strategy-the-default branch from 7ae14ee to 5e8b42c Compare October 13, 2017 07:58
@zvirja zvirja merged commit 2531eaa into AutoFixture:v4 Oct 13, 2017
@zvirja zvirja deleted the make-fixed-name-strategy-the-default branch October 13, 2017 08:07
@zvirja
Copy link
Member Author

zvirja commented Oct 13, 2017

@Kralizek You can now check how VS Live testing works with the latest AutoFixture drop.

@Kralizek
Copy link
Contributor

@zvirja I don't have concrete evidences but I believe that using the AutoData attributes sometimes causes Live Testing to crash and need manual restart.

@zvirja
Copy link
Member Author

zvirja commented Oct 17, 2017

@Kralizek It's bad 😢 Unfortunately, I don't have VS Enterprise and I don't use NUnit to test this feature by myself. Would be awesome if you could try to collect some diag info to troubleshoot why that happens. That might be also a bug in VS.

@frblondin Since you used this approach for a while - have you observed something similar?

@frblondin
Copy link
Contributor

@Kralizek there can be multiple causes for test framework crashes. I personally encountered many of them:
IFixture creation containing an exception, library loading exceptions depending on AppDomain management of test runner or parallel activation binding redirects... I don't know about live testing since I am using the pro edition.

Is it specific to live testing?
Do you have a callstack?
Have you tried running these tests using NUnit runner successfully? Note that if it crashes also with NUnit runner you can debug it from within Visual Studio. See this stackoverflow question.

There is a 1-month trial for the enterprise edition so if you are stuck, the questions above don't help you, and have a sample project incl. a procedure I could try to debug it.

@Kralizek
Copy link
Contributor

NUnit tests works as they should

  • dotnet test
  • R#
  • Visual Studio Test Explorer

I will try to repro and attach the data I got as soon as possible :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants