-
Notifications
You must be signed in to change notification settings - Fork 323
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
BatchSize Runsettings #550
Conversation
@singhsarab, |
@@ -71,6 +71,11 @@ public static class Constants | |||
public const int DefaultCpuCount = 1; | |||
|
|||
/// <summary> | |||
/// The default batch size. | |||
/// </summary> | |||
public const long DefaultBatchSize = 10; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this public
?
@@ -5,9 +5,6 @@ | |||
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Utilities; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: should move inside namespace?
[TestMethod] | ||
public void TestRequestManagerShouldInitializeConsoleLogger() | ||
{ | ||
var mockLoggerEvents = new DummyLoggerEvents(TestSessionMessageLogger.Instance); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make these instance variables.
(These are copied from earlier code, good to clean up)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with few suggestions.
Did we validate that Discovered tests actually show up in batches in the IDE? May be with a 10k tests projects. |
Changes for making BatchSize configurable via runsettings