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

Stop using [Explicit] #3915

Closed
adamralph opened this issue Jul 8, 2016 · 8 comments
Closed

Stop using [Explicit] #3915

adamralph opened this issue Jul 8, 2016 · 8 comments

Comments

@adamralph
Copy link
Contributor

The Visual Studio test runner does not respect this attribute. It runs all tests, whether marked [Explicit] or not. This adds a lot of friction for contributors since running the test suite results in:

  • a sea of red
  • at least two extremely long running tests
  • two instances of modal forms being shown (the licensing forms)

I'm not sure what the best solution is, but I believe it should be something other than [Explicit]. The VS test runner seems to be the one that most .NET developers use.

The VS runner does, however, respect [Ignore] and in PBot we've recently switched to using that instead. The idea being that in order to run it, you comment out that line. It's not elegant, but it's OK since you anyway have to navigate to the source to run the test. It just means hitting Ctrl+K, Ctrl+C before running.

Perhaps another option would be to put all these tests into a project named something like NServiceBus.ManualTests.

@SimonCropp
Copy link
Contributor

The VS test runner seems to be the one that most .NET developers use.

based on what?

It's not elegant, but it's OK since you anyway have to navigate to the source to run the test. It just means hitting Ctrl+K, Ctrl+C before running.

"not elegant" is an understatement

can u include a link to the bug for mstest ignoring [Ignore]

@adamralph
Copy link
Contributor Author

@SimonCropp

based on what?

Based on the issues I've seen raised in other projects I'm involved with. It was incorrect to say "most". I should have said "a significant proportion".

"not elegant" is an understatement

Agreed 😢

can u include a link

I thought I'd start over at NUnit first: nunit/nunit#1664

@adamralph
Copy link
Contributor Author

I've been redirected to nunit/nunit3-vs-adapter#47 (comment)

@danielmarbach
Copy link
Contributor

These are the remaining explicits with a reason. The argument convention test is tracked with #3936 and followup PRs

C:\particular\NServiceBus\src\NServiceBus.Core.Tests\AssemblyScanner\When_directory_with_no_reference_dlls_is_scanned.cs(11):        [Explicit("TODO: re-enable when we make message scanning lazy #1617")]
  C:\particular\NServiceBus\src\NServiceBus.Core.Tests\Config\When_using_convention_based_messages.cs(9):        [Explicit("//TODO: re-enable when we make message scanning lazy #1617")]
  C:\particular\NServiceBus\src\NServiceBus.Core.Tests\Unicast\HandlerInvocationCache.cs(12):    [Explicit("Performance Tests")]
  C:\particular\NServiceBus\src\NServiceBus.AcceptanceTests\PerfMon\CriticalTime\When_CriticalTime_enabled.cs(14):        [Explicit("Since perf counters need to be enabled with powershell")]
  C:\particular\NServiceBus\src\NServiceBus.AcceptanceTests\PerfMon\CriticalTime\When_deferring_a_message.cs(16):        [Explicit("Since perf counters need to be enabled with powershell")]
  C:\particular\NServiceBus\src\NServiceBus.AcceptanceTests\PerfMon\CriticalTime\When_slow_with_CriticalTime_enabled.cs(14):        [Explicit("Since perf counters need to be enabled with powershell")]
  C:\particular\NServiceBus\src\NServiceBus.AcceptanceTests\PerfMon\SLA\When_sending_slow_with_SLA_enabled.cs(15):        [Explicit("Since perf counters need to be enabled with powershell")]
  C:\particular\NServiceBus\src\NServiceBus.AcceptanceTests\PerfMon\SLA\When_sending_with_SLA_enabled.cs(15):        [Explicit("Since perf counters need to be enabled with powershell")]

@adamralph
Copy link
Contributor Author

adamralph commented Jul 22, 2016

My NUnit enquiry led to nunit/nunit-vs-adapter#125 which led to https://visualstudio.uservoice.com/forums/121579-visual-studio-2015/suggestions/15312648-pass-intent-rather-than-a-list-of-tests-to-test

I don't think it's going to get fixed any time soon.

My immediate problem is gone. A new version of TestDriven.NET was released yesterday which recognises our NUnit 3.2.1 tests so I'm using that now.

The only thing that remains then is whether we want to help contributors which are using the VS test runner. In it's current form, the VS test runner is quite unusable for working with any solution which has a large number of tests, and tests which are explicit by virtue of them being long running, since the only way to run any group of tests and not the explicit ones is to run all tests in the solution.

@andreasohlund
Copy link
Member

With PerfCounters gone in v7 we only have a few left

C:\dev\NServiceBus\src\NServiceBus.Core.Tests\ArgumentExceptionTests.cs(13): [Explicit]
C:\dev\NServiceBus\src\NServiceBus.Core.Tests\Config\When_using_convention_based_messages.cs(9): [Explicit("//TODO: re-enable when we make message scanning lazy #1617")]
C:\dev\NServiceBus\src\NServiceBus.Core.Tests\Unicast\HandlerInvocationCache.cs(12): [Explicit("Performance Tests")]
C:\dev\NServiceBus\src\NServiceBus.AcceptanceTests\Core\PerfMon\CriticalTime\When_CriticalTime_enabled.cs(15): [Explicit("Since perf counters need to be enabled with powershell")]
C:\dev\NServiceBus\src\NServiceBus.AcceptanceTests\Core\PerfMon\CriticalTime\When_deferring_a_message.cs(18): [Explicit("Since perf counters need to be enabled with powershell")]
C:\dev\NServiceBus\src\NServiceBus.AcceptanceTests\Core\PerfMon\CriticalTime\When_slow_with_CriticalTime_enabled.cs(15): [Explicit("Since perf counters need to be enabled with powershell")]
C:\dev\NServiceBus\src\NServiceBus.AcceptanceTests\Core\PerfMon\SLA\When_sending_slow_with_SLA_enabled.cs(16): [Explicit("Since perf counters need to be enabled with powershell")]
C:\dev\NServiceBus\src\NServiceBus.AcceptanceTests\Core\PerfMon\SLA\When_sending_with_SLA_enabled.cs(16): [Explicit("Since perf counters need to be enabled with powershell")]

@adamralph how strongly do you feel for this? (Close?)

@adamralph
Copy link
Contributor Author

@andreasohlund I defer to my previous comment:

My immediate problem is gone. A new version of TestDriven.NET was released yesterday which recognises our NUnit 3.2.1 tests so I'm using that now.

The only thing that remains then is whether we want to help contributors which are using the VS test runner. In it's current form, the VS test runner is quite unusable for working with any solution which has a large number of tests, and tests which are explicit by virtue of them being long running, since the only way to run any group of tests and not the explicit ones is to run all tests in the solution.

I'll leave it up to @Particular/nservicebus-maintainers to judge the importance of the last point.

@andreasohlund
Copy link
Member

@Particular/nservicebus-maintainers please reopen if needed

@andreasohlund andreasohlund modified the milestone: Future Mar 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants