-
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
Exceptions flow to Translation layer #1434
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -111,7 +111,6 @@ internal UriDataAttachment Clone(string baseDirectory, bool useAbsoluteUri) | |
{ | ||
Debug.Assert(!string.IsNullOrEmpty(baseDirectory), "'baseDirectory' is null or empty"); | ||
Debug.Assert(baseDirectory == baseDirectory.Trim(), "'baseDirectory' contains whitespace at the ends"); | ||
Debug.Assert(Path.IsPathRooted(baseDirectory), "'baseDirectory' is not a rooted path"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assertion if to test assumption, here it says that this method expects basedirectory to be rooted, if that is not the case, then may be change to to that it should not be null or empty. |
||
|
||
if (useAbsoluteUri != this.uri.IsAbsoluteUri) | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -329,11 +329,19 @@ private bool ExecuteArgumentProcessor(IArgumentProcessor processor, ref int exit | |
} | ||
catch (Exception ex) | ||
{ | ||
if (ex is CommandLineException || ex is TestPlatformException || ex is SettingsException) | ||
if (ex is CommandLineException || ex is TestPlatformException || ex is SettingsException || ex is InvalidOperationException) | ||
{ | ||
EqtTrace.Error("ExecuteArgumentProcessor: failed to execute argument process: {0}", ex); | ||
this.Output.Error(false, ex.Message); | ||
result = ArgumentProcessorResult.Fail; | ||
|
||
// Send inner exception only when its message is different to avoid duplicate. | ||
if (ex is TestPlatformException && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of this we can do ex.ToString() in line 335 which will print inner exception. But in that case duplicacy will be there if inner exception and outer exception message are same. This happens for many scenarios in TestPlatformException |
||
ex.InnerException != null && | ||
!string.Equals(ex.InnerException.Message, ex.Message, StringComparison.CurrentCultureIgnoreCase)) | ||
{ | ||
this.Output.Error(false, ex.InnerException.Message); | ||
} | ||
} | ||
else | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,28 +1,22 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
||
using System.ComponentModel; | ||
using System.Xml; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: move these as well below? |
||
using Microsoft.VisualStudio.TestPlatform.Common; | ||
using Microsoft.VisualStudio.TestPlatform.Common.Utilities; | ||
|
||
namespace Microsoft.VisualStudio.TestPlatform.CommandLine.Processors | ||
{ | ||
using Microsoft.VisualStudio.TestPlatform.Common.Logging; | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Diagnostics.Contracts; | ||
using System.Globalization; | ||
|
||
using Microsoft.VisualStudio.TestPlatform.CommandLine.Processors.Utilities; | ||
|
||
using CommandLineResources = Microsoft.VisualStudio.TestPlatform.CommandLine.Resources.Resources; | ||
using Microsoft.VisualStudio.TestPlatform.CommandLine.Internal; | ||
using Microsoft.VisualStudio.TestPlatform.Client; | ||
using Microsoft.VisualStudio.TestPlatform.Common.Interfaces; | ||
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Utilities; | ||
using Microsoft.VisualStudio.TestPlatform.ObjectModel; | ||
using System.Collections.ObjectModel; | ||
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Utilities; | ||
using CommandLineResources = Microsoft.VisualStudio.TestPlatform.CommandLine.Resources.Resources; | ||
|
||
/// <summary> | ||
/// An argument processor that allows the user to enable a specific logger | ||
|
@@ -218,7 +212,7 @@ public static void AddLoggerToRunSettings(string loggerArgument, IRunSettingsPro | |
|
||
// Remove existing logger. | ||
var existingLoggerIndex = loggerRunSettings.GetExistingLoggerIndex(logger); | ||
if (existingLoggerIndex > 0) | ||
if (existingLoggerIndex >= 0) | ||
{ | ||
loggerRunSettings.LoggerSettingsList.RemoveAt(existingLoggerIndex); | ||
} | ||
|
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.
Same goes here, if can pick attachments, from relative URI, the change it that this path should not be null/empty