Reformat code in ClousotTests #152
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,5 @@ | ||
| // Copyright (c) Microsoft. All rights reserved. | ||
| // Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
|
|
@@ -20,147 +9,146 @@ | |
|
|
||
| namespace Tests | ||
| { | ||
| public class AsyncTestDriver | ||
| { | ||
| private delegate void IsolatedAction<in T>(T obj, out Exception exceptionThrown, out string dataReceived); | ||
|
|
||
| public static readonly uint MaxWaitHandles_Default = Math.Max(1, Math.Min(4, (uint)(Environment.ProcessorCount - 1))); | ||
| public static readonly uint MaxWaitHandles_AllButOne = Math.Max(1, (uint)(Environment.ProcessorCount - 1)); | ||
|
|
||
| private static readonly int SingleTestMaxWait = 200000; | ||
|
|
||
| private readonly Action<Options, Output> action; | ||
| private readonly IsolatedAction<Options> actionDelegate; | ||
| private readonly Func<Options, bool> skipTest; | ||
| private Dictionary<string, IAsyncResult> testAsyncResults; | ||
| private readonly uint maxWaitHandles; | ||
| private WaitHandle[] waitHandles; | ||
| private int nbWaitHandles; | ||
| private bool beginTestsProcessed = false; | ||
| private bool orderReversed = false; | ||
|
|
||
| public string BeginMessage; | ||
|
|
||
| public AsyncTestDriver(Action<Options, Output> action, Func<Options, bool> skipTest) | ||
| : this(action, skipTest, MaxWaitHandles_Default) | ||
| { } | ||
|
|
||
| public AsyncTestDriver(Action<Options, Output> action, Func<Options, bool> skipTest, uint maxWaitHandles) | ||
| { | ||
| this.action = action; | ||
| actionDelegate = this.ActionAsIsolated; | ||
| this.skipTest = skipTest; | ||
| this.maxWaitHandles = maxWaitHandles; | ||
| } | ||
|
|
||
| // We have no control on the order of the tests, so we make sure | ||
| // to always call Begin before End | ||
|
|
||
| public void BeginTest(Options options) | ||
| { | ||
| if (skipTest(options)) | ||
| return; | ||
|
|
||
| beginTestsProcessed = true; | ||
|
|
||
| if (orderReversed) | ||
|
Contributor
There was a problem hiding this comment. Can formatter insert braces for single statements like that?
Contributor
Author
There was a problem hiding this comment. This is following the .NET foundation bracing rules, which generally requires braces but makes an exception for cases where the entire block is a single statement which does not span multiple lines. This bracing rule was later implemented in StyleCopAnalyzers as well (DotNetAnalyzers/StyleCopAnalyzers#716, DotNetAnalyzers/StyleCopAnalyzers#717).
Contributor
There was a problem hiding this comment. There is a plenty of places in this code base like: if (check)
{
// several statements
}
else
oneStatement;This is very unreadable. I would prefer to not use braces only on a simple if statements (like old-style preconditions) but only when there is no
Contributor
Author
There was a problem hiding this comment. No, if the
Contributor
There was a problem hiding this comment. It seems I didn't communicate my thoughts correctly. I'll try once more. My rules: use curly braces in all cases. The only exception is simple if-statements without else blocks for short one liners like argument validation.
Contributor
Author
There was a problem hiding this comment. StyleCopAnalyzers and StyleCop do not currently have a rule to enforce this, though one could certainly be proposed. However, there are a couple additional things to keep in mind:
Contributor
There was a problem hiding this comment. Actually, I'm just curios how far my personal preferences from proposed once:) And yep, moving to dotnet organization makes a lot of sense. I'll talk to someone to understand how to do this. |
||
| this.EndTestInternal(options); | ||
| else | ||
| this.BeginTestInternal(options); | ||
| } | ||
|
|
||
| public void EndTest(Options options) | ||
| { | ||
| if (skipTest(options)) | ||
| return; | ||
|
|
||
| if (!beginTestsProcessed) | ||
| orderReversed = true; | ||
|
|
||
| if (orderReversed) | ||
| this.BeginTestInternal(options); | ||
| else | ||
| this.EndTestInternal(options); | ||
| } | ||
|
|
||
| private void BeginTestInternal(Options options) | ||
| { | ||
| try | ||
| { | ||
| if (testAsyncResults == null) | ||
| testAsyncResults = new Dictionary<string, IAsyncResult>(); | ||
|
|
||
| if (waitHandles == null) | ||
| waitHandles = new WaitHandle[maxWaitHandles]; | ||
|
|
||
| var index = nbWaitHandles; | ||
| if (index == waitHandles.Length) | ||
| { | ||
| index = WaitHandle.WaitAny(waitHandles, waitHandles.Length * SingleTestMaxWait); | ||
| Assert.AreNotEqual(index, WaitHandle.WaitTimeout, "Previous tests timed out"); | ||
| nbWaitHandles--; | ||
| } | ||
|
|
||
| Exception dummyOutException; | ||
| string dummyOutString; | ||
| var asyncResult = actionDelegate.BeginInvoke(options, out dummyOutException, out dummyOutString, null, null); | ||
| testAsyncResults.Add(options.TestName, asyncResult); | ||
| waitHandles[index] = asyncResult.AsyncWaitHandle; | ||
| nbWaitHandles++; | ||
|
|
||
| Console.WriteLine(this.BeginMessage); | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| Console.WriteLine("EXCEPTION: {0}", e.Message); | ||
| Assert.Fail("Exception caught"); | ||
| } | ||
| } | ||
|
|
||
| private void EndTestInternal(Options options) | ||
| { | ||
| Assert.IsNotNull(testAsyncResults, "Begin part of the test not selected"); | ||
|
|
||
| IAsyncResult asyncResult; | ||
| if (!testAsyncResults.TryGetValue(options.TestName, out asyncResult)) | ||
| Assert.Fail("Begin part of the test not run"); | ||
|
|
||
| testAsyncResults.Remove(options.TestName); | ||
|
|
||
| Assert.IsTrue(asyncResult.AsyncWaitHandle.WaitOne(SingleTestMaxWait), "Test timed out"); | ||
|
|
||
| Exception exceptionThrown; | ||
| string dataReceived; | ||
| actionDelegate.EndInvoke(out exceptionThrown, out dataReceived, asyncResult); | ||
|
|
||
| Console.WriteLine(); | ||
| Console.WriteLine("This test case was performed {0}synchronously", asyncResult.CompletedSynchronously ? "" : "a"); | ||
| Console.WriteLine(); | ||
|
|
||
| Console.Write(dataReceived); | ||
| if (exceptionThrown != null) | ||
| throw exceptionThrown; | ||
| } | ||
|
|
||
|
|
||
|
|
||
| private void ActionAsIsolated(Options options, out Exception exceptionThrown, out string dataReceived) | ||
| { | ||
| using (var stringWriter = new StringWriter()) | ||
| { | ||
| var output = new Output(String.Format("Isolated::{0}", options.TestName), stringWriter); | ||
| exceptionThrown = null; | ||
| try | ||
| { | ||
| action(options, output); | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| exceptionThrown = e; | ||
| } | ||
| dataReceived = stringWriter.ToString(); | ||
| } | ||
| } | ||
| } | ||
| } | ||
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.
I think I saw that in some other PRs with formatting changes you also changed field names...
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.
I explicitly disabled that - I left a message in Gitter explaining why (basically there is so much code to reformat that we need to do the safe option first and then we can consider renaming things in the future).
I closed all the other formatting PRs so I can update them to also not rename fields.
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.
Sounds good. Jut wanted to double check.