-
Notifications
You must be signed in to change notification settings - Fork 325
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
Entire test runner workflow is too slow and unenjoyable #623
Comments
6a. Please update the icon used for "test currently running". Ideally to an icon that moves, but at least to an icon that is not mainly green the same color indicating passed tests. This also make the currently running test very hard to find, especially if it is in a list of tests you already ran and passed.
|
@issafram Wouldn't it be great if |
For the things which appear to be xUnit.net-centric:
|
The problem is I'm not trying to |
I completely agree. I have been always staying out of using XUnit for this sole reason (+ another one that when using output capturing and you have lots of output, XUnit is damn slow - or with its resharper integration, not sure... - while NUnit works perfectly there) |
I would add one more to that list: not only is discovery really, really slow - to even get to the discovery phase when opening a Solution takes > 1 minute on http://github.com/dotnet/roslyn-project-system. I suspect waiting for output groups from CPS to populate is probably a lot of that - but I've not investigated. |
If I remember correctly about the "current" running test, I filed this back in VS 2012 when the Test Explorer was added, the reason they don't show current progress was the communication back to VS to update the "current" test was slowing down the test run. However, I really only want to see "current" when the test > a couple of seconds, by which case you could make it to only communicate "current" when running tests that takes a while. |
@bradwilson A thought about the Console problem - what if you set your own writer for Console (via SetOut) that stored it's data based on |
@davidfowl great feedback. Taking up the test window ones:
This could be related to dotnet/project-system#62? /cc @davkean
It is possible to do parallel discovery with "Parallel" option enabled in Test Explorer. #499 is tracking if discovery should be parallel by default.
This is an usability issue. Created #626. Test explorer shows the
Created #627 to track this. Thanks again for bringing this up. Request readers to vote the individual issues (it will help us prioritize). |
I do have to chime in here and say that I have yet to see a unit test experience that I really enjoy. Even JetBrains which is usually pretty good with their .NET implementations/improvements really misses the mark in their efforts here with their dotCover product. I might be off the usual beaten path here, but I think at a minimum there should be a grid view of some sort for any test viewer that allows us to easily grok the state of our current test state. This of course would not only cover the display of the obvious expected fail/pass expectations, but also all the cool metadata and statistics that are very difficult to discover and understand as currently presented. Plus, I think it would attend to at least some of the points that Mr. Fowler presents in this issue. So, very much in favor of seeing any innovation here. Btw @davkean, in addition to your solid suggestion, @gfraiteur and the talented PostSharp team are working on a fully async-aware and capable logging solution known as Frigate. I saw it during a recent webinar and immediately thought of the challenges that @bradwilson once described (to put it kindly 😄) in having to deal with logging within an async environment. Something like this would be very useful for logging in async-based testing, if by the very least by module/extension. Definitely at least worth knowing about, IMO. 👍 |
@davkean @bradwilson @Mike-EEE Mike, thanks for mentioning Frigate. The project is still very early, but if you want to know about it, you can watch the webinar recording from the 19th minute. Don't hesitate to ping me with questions or feedback. I'm happy to chat about integration with test runners. |
@bradwilson I added an issue with examples for changes to xunit that would allow ITestOutputHelper to be registered for all tests without the need for explicit constructor injection. The changes don't affect existing functionally, just provide better subclassing opportunities for those who want/need it. Sorry to partially hijack this issue! See xunit/xunit#1119 |
Yes it would, but this is very similar to the Microsoft Build & Release Management Team deciding that
I like to use the adapter pattern for this issue. I use an I then setup my unit tests to call XUnit's I know it sounds painful, but you are basically hard coding a dependency with I'm not saying that I agree with the decision to disallow the Console from testing, but I'm ok with making my code more flexible. |
@codito, for 2.
This is a real pain point on large desktop solutions as well (Roslyn). A simple change in one project causes 3-5 minutes of discovery time (which is ridiculous considering the console does it in milliseconds). |
For 6, xUnit has long running test detection but it's off by default. Add a config value called |
tagging @sbaid |
Often also leaves lots of dotnet processes hanging around |
cc @jcouv As ref-assembly generation may provide some benefits here. |
@tannergooding wrote:
There's a detailed RCA on the discovery perf at #485. Here's the current action plan we're working on:
Can you share the repro steps (project, sln) mentioned above? (2) is already available today, we can try and evaluate the benefits. It will be great to get some feedback on (3), do people find auto test discovery in test explorer useful? |
On the names/width issue; since the names are hierarchical, this seems to be a perfect opportunity for an implicit tree structure, perhaps grouped so that you don't have 5 levels with only one sub-level and nothing else; so in your example there would be just one parent node of
With nodes underneath that for every separate thing with a different FQN, and the test methods as the leaf level Relatively simple UI change that can be done purely on the text of the FQN (no input from the provider needed), and could make it so much more usable |
@codito, Repro Steps:
I'm also not sure that reading source information is the slow part here, I'm pretty sure its the serialization mechanism between the test adapter and VS. xUnit can discover all 60k tests in Roslyn in seconds (by default its just reflecting over all attributes that inherit from You can also crack the PDBs and pull out the source information for the tests in a very small amount of time (seconds) as well (and if this was a bottleneck, then it could be made lazy and only pulled/displayed when the user selects an individual testcase, either for navigation or for viewing details). With both of those problems "gone", the remaining issues are:
|
That's not quite accurate. xUnit never uses Reflection to statically look at tests. It always executes the discoverer's, which can happen to be facts/theories, in the assembly under test. That's core to how xUnit is so flexible, you can provide custom discovery/execution mechanisms directly in the assembly under test. |
@onovotny, you're correct, just didn't explain myself very well 😄. I meant that the default discoverers (asumming you don't have any custom discoverers) appear to use reflection over attributes to locate the tests. Customer discoverers can do whatever mechanism they wish. In either case, xUnit is able to discover all 60k of the Roslyn tests in seconds (not minutes), so discovery itself is not the likely bottleneck. |
@tannergooding try opening Task manager next time and killing VsTest.Console while you are waiting... |
This just happened to me, opened a largish solution, discovered all test. Then I clicked Run All, and it proceeded to rediscover all tests. |
@tannergooding I am able to reproduce it. Here are the numbers and steps to try out the "parallel discovery" in Desktop targeted projects:. 1. Without any changes, I see following elapsed time for discovery roslyn.sln: 2. With changes below, this is how the numbers look (~1/5th of earlier elapsed time): 3. With project modification:
Observation
Steps to tryout parallel discovery
Tests will start discovering. You may hit #76 though.
The existing logic in Test Explorer should already do this. It assumes build will only compile projects that have changed. It diffs the timestamp of earlier Test Containers; triggers discovery only for the Test Container whose timestamp has changed after the build operation. If you have a repro (for Desktop targets) that violates this, we will be happy to look into it. Number of containers sent for rediscovery should be equal to number of dependent test projects impacted by the change in a project.
By default, Desktop tests run via older Test Platform. There is no JSON involved. The changes I've suggested in (2) above use the newer test platform (which uses JSON). I am not sure if we can change the protocol to binary, it is documented and public for consumption by various IDE/Editors (the response from test host is directly relayed back to Editors). There are several optimizations possible though (reducing JSON verbosity, using streams/custom serializers). They are tracked at https://github.com/microsoft/vstest/issues?utf8=%E2%9C%93&q=is%3Aissue%20is%3Aopen%20label%3Aperf |
@codito Is anyone working on fixing any of the issues mentioned? How much of it is open sourced? Maybe the community can help with some of it. |
First time VS Test user, long time R# user. Started with Literally all the points raised impact us here too :( Even something as simple and similar as David's point 3 - run/debug current test which the cursor is flashing, in .. or right click in that test method (code) and run/debug. Here's some sample screenies from R#. No, this is not trying to be 'MS is copying another product and trying to destroy it" .. but .. please be inspired by this premium product. A number of us feel that we just can't use the built in VS Test while these features aren't around. Sure we have paid R# lic's. But we find R# really slows down our VS experience (it does SOOO much, to be fair to it .. just tooo much for us) .. which is why we would love to see if we can start using VS Test instead. This project should please be OpenSourced. Plz. Let the community help! |
Ah, misread your second image, apologies. :) |
This does appear to be a significant bottleneck for LUT runs. With every LUT run ( which we do after every edit) here is what we are seeing:
Additionally. I think VS process will also have these memory overhead that I have mentioned below. I do understand why you guys have designed it the way it is ( to make it work for all scenarios) , however to deliver the performance that LUT needs, we do need a by-pass from this. It should be an alternative that is available to TP clients. if the existing mechanism is too bulky for their scenarios. We ( @genlu ) do have perf traces which show these bottlenecks pretty clearly. We made changes to the Test Platform code to collect this information. Please let us know if you would like to see them. @sbaid - we would need this to address perf and scale issues for LUT for the quarterly release. Please let us know if you want us to file a separate issue for this. |
@ManishJayaswal I will follow up with you offline to get the traces. |
Thank you for all the feedback and discussions. Here is a summary of the concerns raised in this thread. I have pointed to the relevant issue tracking it. 1. The test explorer window is too narrow and test names are unreadable by default. 2. The test runner rebuilds the test project and it's dependencies when running any test. This is horrible experience in .NET Core projects because of the missing of "up to date" check. The entire project graph is checked for stale assets to run a single test. 3. I'm unable to right click a specific test in source and run the test. It seems to run all tests in that class instead of running the specific test method I targeted. This used to work before the move to csproj and has been broken with the new test runner and tooling. 4. Test discovery is super slow across multiple projects. It seems as though test discovery happens sequentially for each project in the solution instead of in parallel. Why is that? 5. The test runner inside visual studio doesn't show console output. I want to see my Console.WriteLine output in the test runner window … 6. I can't see the active test being run. There are situations where I want to see the current test being run (both on the command line and in visual studio) so I can tell which one is hanging (if it hangs). It would have saved me so many hours if there was a simple way to show the active test being run in both the test explorer and on the command line. 7. Certain crashes make the test runner implode. I'm working on a refactoring and when I run tests sometimes this happens: 8. Is anyone working on fixing any of the issues mentioned? How much of it is open sourced? Maybe the community can help with some of it. 9. Group by namespace 10. Run all or just several tests from Test Explorer under performance profiler Test Explorer backlog |
@pvlakshm or @codito - how about my questions about:
Ref: Also .. today we were trying to use it again and had this issue: We ran all tests (say 50 or 100). A test (or two? three?) errored. We saw the "red error bar" at the top but had no idea which actual test errored. Here's a simple example... Which test has errored? Then compare this to a tool that makes things easier to work with: Not hating at all - just trying to help and hope this feedback (under the OP's same topic) can be addressed :) 🙏 |
Please see the summary above. Since there were some issues not related to the Test Explorer per se, I have created a new issue #725 to track Test Explorer specific improvements. |
One thing that vexes me every time I try vs test runner instead of resharper is the global 'default processor architecture'. I don't understand why it doesn't appear to be a default, but actually the architecture it runs all tests in. Resharper in contrast looks at the architecture of the test project and uses that on a per project basis. It might be useful if the guys who own this component tried resharper unit testing for a while to appreciate the massive gaps in useability. |
+1 to test runner being suboptimal. Forced to use it due to bug in current Resharper and boy its so slow and annoying. In addition to things said the test discovery exe keeps locking xunit dlls, so I had to add a prebuild step to kill it. Can't wait for R# to fix the bug so I can use again, but it actually would be nice if VS wasn't such an unproductive experience. |
Yeah, this is unusable. It builds every single time there is a test. I'm looking at a minute and a half before it runs my single test that is: Assert.True(1 == 1). |
01.12.2017 Sill valuable feedback, even with all the performance optimizations made. Add option in Options menu to specify where the log file should be rather than enabling it with config file editing. |
@soeron Thanks for feedback, We have issue to improve enable logs internally (https://devdiv.visualstudio.com/DevDiv/VS.in%20Agile%20Testing%20IDE/_queries/edit/531191/?triage=true), I create one here #1315 for tracking.
Totally agreed 👍. Created an issue here #1316 .
Can you create a separate issue for it with logs and repro project? /cc @pvlakshm @cltshivash @ManishJayaswal for triage #1315 #1316 |
Regarding the public class DefaultConsole : IConsole, IFallbackImplementationFor<IConsole>
{
public void WriteLine(string s) => Console.WriteLine(s);
} |
Copied the issue to developer community. Hence closing this |
As some of you know, I'm on a mission to be able to stop using ReSharper. Without a doubt the most frequent reason I need to keep turning ReSharper back on is the convenience of these things:
Of the several testing UIs I've seen, Visual Studio is the only one that lacks the intuitive hierarchical paradigm. When trying to run an arbitrary selection of tests repeatedly, scrolling through a massive flat list with near-uselessly-primitive grouping abilities is distracting and tediously manual. It leaves me switching ReSharper or NCrunch back on so that I feel like I can focus on coding. I genuinely believe that the VS teams want to provide a focused, considerate tool out of the box. You wouldn't want to blindly copy another tool. This is one of the few areas in which I can say urgently that specific other tools have gotten it very right; thus this feedback. 😃 |
Hey all, I just wanted to give an update on the progress we've made in this experience. We still have a long way to go, but here are some improvements you can see in Visual Studio 2017 Update 15.6.
We've added a hierarchy view that should help the readability of test names as well as navigation. The Projects, namespaces, and classes are different tiers in the hierarchy.
Real Time Test Discovery is on by default in Update 15.6 and should improve this greatly. This discovers tests from source code instead of by built assemblies and is much faster. |
@kendrahavens Thanks for the update and I really don't want to add another +1 comment to a github thread .. but ... just adding the hierarchy view has been such a godsend! Testing is already a bazillion (yes, I used scientific metrics to calculate the improvement level) times easier to manage. Thank you for listening! Looking forward to the other improvements that hopefully coming too 🥂 |
With apologies for the thread necromancy (this issue is still open after-all), but is there any "fix" for how the Test Explorer window automatically moves to a vertical-split when its made wider? (The whole point for making the Test Explorer window wider is to see more of the test-list section!) - David Fowler reported it in his original posting in 2017 and it's still a problem today:
As I'm currently using Also, right-clicking on the top few nodes in the explorer when tests are grouped by namespace and class (with xUnit at least, I don't think it affects MSTest but I might be wrong) has a 2-3 second delay before the context-menu actually appears - this is consistently reproducible on different machines. Right-clicking on a test node results in the menu appearing near-instantly (but still with a subtle delay). |
It's been a long beat since we've given an update on this ticket. Almost every experience request should be addressed in the past several Visual Studio and VS Test Platform updates. Let me give an roll call:
Going forward, there is still lots to fix in the testing experience, but I'd suggest we open a fresh issues for the individual features and close this one. I'd like to thank the community for following, upvoting, and commenting their scenarios! Hopefully most of you see a night and day difference between the test experience of 2017 and what we have today. (And if you don't, please keep the feedback coming!) Also, a note on this repo, the vstest repo specifically refers to the VSTest Platform. The majority of Visual Studio Test Explorer feedback should be submitted on https://developercommunity.visualstudio.com/ with the Provide Feedback Tool. in Visual Studio. This distinction is because the Test Explorer code is closed source and not a part of this repo. Thank you again for helping us improve this experience for the millions of developers who use this tool! @Jehoel I believe we will be able to look at adding a splitter option in the next few sprints. Please follow this developer community ticket: "Group Summary" section in "Test Explorer" moves. If you are still experiencing perf issues, I’d also encourage you to file a bug on developer community. |
5.2) I experimented with using AsyncLocal (as suggested above) to preserve the output per test in MSTest, and found this amazing XUnit extension by @SimonCropp that already implements it for XUnit. https://github.com/SimonCropp/XunitContext#xunitcontextbase All you need to do is install the XUnitContext package and use the provided base class or register the context yourself. Then all Console writes (and other sources as well), will be written to the output stream per test and echoed in the test output when the test fails. Here output from two tests running in parallel: As you can hopefully see in the above output, the results are written to their respective tests even though the tests are running in parallel. The nice thing is that even non-awaited task that finishes while the test is still running will write into the correct output. There are 3 more edge cases, that I was able to identify:
I was able to capture 2 and 3, in my extension because I used a global setup to replace the Console logger before any test run. That said we don't have a way of showing that output in TestExplorer at the moment anyway, but it might be an improvement to consider, and get it at least into logs. Each of the edge cases has an example in the code below, in case @SimonCropp would like to comment, or correct my usage of his nuget package. using System;
using System.Diagnostics;
using System.Threading;
using System.Threading.Tasks;
using Xunit;
using Xunit.Abstractions;
namespace TestProject1
{
public static class TestContext
{
public static bool Flag = true;
public static Stopwatch Sw = Stopwatch.StartNew();
}
public class TestedClass
{
public async Task Method1(string id)
{
Console.WriteLine($"{TestContext.Sw.ElapsedMilliseconds:0000} method1 called from {id}");
await Method2(id);
}
public Task Method2(string id)
{
Console.WriteLine($"{TestContext.Sw.ElapsedMilliseconds:0000} method2 called from {id}");
// not awaited on purpose
Task.Run(() => { Thread.Sleep(200); Console.WriteLine($"{TestContext.Sw.ElapsedMilliseconds:0000} Non-awaited short task finished. {id}."); });
// this will never show in output because the process terminates before this task can finish
Task.Run(() => { Thread.Sleep(10000); Console.WriteLine($"{TestContext.Sw.ElapsedMilliseconds:0000} Non-awaited long task finished. {id}."); });
using (ExecutionContext.SuppressFlow())
{
// this won't show up in the test result because the flow of dependencies was suppressed
Task.Run(() => Console.WriteLine($"{TestContext.Sw.ElapsedMilliseconds:0000} - Supressed. {id}"));
}
return Task.CompletedTask;
}
}
public class UnitTest1 : IDisposable // Register the outputs yourself.
{
public UnitTest1(ITestOutputHelper output)
{
XunitContext.Register(output);
}
public void Dispose()
{
XunitContext.Flush();
}
[Fact]
public async Task TestMethod1()
{
Console.WriteLine($"{TestContext.Sw.ElapsedMilliseconds:0000} t1-start");
var tc = new TestedClass();
await tc.Method1("t1");
// wait for the other test so we know there are tests running in parallel
while (TestContext.Flag)
{
Console.WriteLine($"{TestContext.Sw.ElapsedMilliseconds:0000} t1-wait");
await Task.Delay(100);
}
await tc.Method1("t1");
Console.WriteLine($"{TestContext.Sw.ElapsedMilliseconds:0000} t1-end");
// wait for the short non-awaited tasks to complete to see if they print in the correct
// test output, if the test happens to run long enough
await Task.Delay(500);
throw new Exception("err");
}
}
public class UnitTest2 : XunitContextBase // Or use the provided base class
{
public UnitTest2(ITestOutputHelper output) : base(output)
{
}
[Fact]
public async Task TestMethod2()
{
Console.WriteLine($"{TestContext.Sw.ElapsedMilliseconds:0000} t2-start");
var tc = new TestedClass();
await tc.Method1("t2");
await Task.Delay(1000);
TestContext.Flag = false;
Console.WriteLine($"{TestContext.Sw.ElapsedMilliseconds:0000} t2-end");
Task.Run(() => { Thread.Sleep(200); Console.WriteLine($"{TestContext.Sw.ElapsedMilliseconds:0000} Non-awaited short task finished after test t2 finished."); });
throw new Exception("err");
}
}
} |
I'm on the ASP.NET team and using the VS 2017 Community RTM build and CLI tools. There are a few key issues that totally disrupt my workflow with the test runner and generally make the experience unenjoyable. We use xunit with the test explorer and here are the issues I experience daily:
The test explorer window is too narrow and test names are unreadable by default.
Test names contain the full namespace which make it impossible to fit into the tiny winow. After I expand it to see actual, it moves the summary window to the side (which I never wanted to begin with):
The test runner rebuilds the test project and it's dependencies when running any test. This is horrible experience in .NET Core projects because of the missing of "up to date" check. The entire project graph is checked for stale assets to run a single test.
I'm unable to right click a specific test in source and run the test. It seems to run all tests in that class instead of running the specific test method I targeted. This used to work before the move to csproj and has been broken with the new test runner and tooling.
Test discovery is super slow across multiple projects. It seems as though test discovery happens sequentially for each project in the solution instead of in parallel. Why is that?
The test runner inside visual studio doesn't show console output. I want to see my
Console.WriteLine
output in the test runner window. I use it to debug race conditions all the time. Sometimes using the debugger doesn't cut it because it slows everything down to a point where it's hard to reproduce the problem. There are developers are my team that built a hacked up version of the test runner to enable this on their dev boxes. This is a real blocker.I can't see the active test being run. There are situations where I want to see the current test being run (both on the command line and in visual studio) so I can tell which one is hanging (if it hangs). It would have saved me so many hours if there was a simple way to show the active test being run in both the test explorer and on the command line.
Certain crashes make the test runner implode. I'm working on a refactoring and when I run tests sometimes this happens:
Which probably means the process crashed but the test runner doesn't help me diagnose things here. I don't know which test was running when it crashed, so I have to manually binary search until I find the one that is the culprit.
I've been actively using this product for the last week in-depth and I feel like a few simple tweaks could really make this much much better.
The text was updated successfully, but these errors were encountered: