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

Map incoming and outgoing requests #3314

Merged
merged 15 commits into from
Feb 22, 2022
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;

/// <summary>
/// This interface holds additional values for a request handler when rewriting paths is used.
/// This is in case when --local-path and --remote-path parameters are provided and testhost is running
/// in a remote deployment. This interface is used only to avoid changes to public API of TestRequestHandler.
/// </summary>
internal interface IDeploymentAwareTestRequestHandler
{
string LocalPath { get; set; }
string RemotePath { get; set; }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;
using System.Collections.Generic;
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
using ObjectModel;
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
using System.Collections.ObjectModel;
nohwnd marked this conversation as resolved.
Show resolved Hide resolved

internal interface IPathConverter
{
internal string UpdatePath(string path, PathConversionDirection updateDirection);
nohwnd marked this conversation as resolved.
Show resolved Hide resolved

internal IEnumerable<string> UpdatePaths(IEnumerable<string> enumerable, PathConversionDirection updateDirection);
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
nohwnd marked this conversation as resolved.
Show resolved Hide resolved

internal TestCase UpdateTestCase(TestCase testCase, PathConversionDirection updateDirection);

internal IEnumerable<TestCase> UpdateTestCases(IEnumerable<TestCase> testCases, PathConversionDirection updateDirection);

internal TestRunCompleteEventArgs UpdateTestRunCompleteEventArgs(TestRunCompleteEventArgs testRunCompleteEventArgs, PathConversionDirection updateDirection);

internal TestRunChangedEventArgs UpdateTestRunChangedEventArgs(TestRunChangedEventArgs testRunChangedArgs, PathConversionDirection updateDirection);

internal Collection<AttachmentSet> UpdateAttachmentSets(Collection<AttachmentSet> attachmentSets, PathConversionDirection updateDirection);
nohwnd marked this conversation as resolved.
Show resolved Hide resolved

internal ICollection<AttachmentSet> UpdateAttachmentSets(ICollection<AttachmentSet> attachmentSets, PathConversionDirection updateDirection);

internal DiscoveryCriteria UpdateDiscoveryCriteria(DiscoveryCriteria discoveryCriteria, PathConversionDirection updateDirection);

internal TestRunCriteriaWithSources UpdateTestRunCriteriaWithSources(TestRunCriteriaWithSources testRunCriteriaWithSources, PathConversionDirection updateDirection);

internal TestRunCriteriaWithTests UpdateTestRunCriteriaWithTests(TestRunCriteriaWithTests testRunCriteriaWithTests, PathConversionDirection updateDirection);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;
using System.Collections.Generic;
using ObjectModel;
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
using System.Collections.ObjectModel;

internal class NullPathConverter : IPathConverter
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
{
Collection<AttachmentSet> IPathConverter.UpdateAttachmentSets(Collection<AttachmentSet> attachmentSets, PathConversionDirection updateDirection)
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
{
return attachmentSets;
}

ICollection<AttachmentSet> IPathConverter.UpdateAttachmentSets(ICollection<AttachmentSet> attachmentSets, PathConversionDirection updateDirection)
{
return attachmentSets;
}

DiscoveryCriteria IPathConverter.UpdateDiscoveryCriteria(DiscoveryCriteria discoveryCriteria, PathConversionDirection updateDirection)
{
return discoveryCriteria;
}

string IPathConverter.UpdatePath(string path, PathConversionDirection updateDirection)
{
return path;
}

IEnumerable<string> IPathConverter.UpdatePaths(IEnumerable<string> enumerable, PathConversionDirection updateDirection)
{
return enumerable;
}

TestCase IPathConverter.UpdateTestCase(TestCase testCase, PathConversionDirection updateDirection)
{
return testCase;
}

IEnumerable<TestCase> IPathConverter.UpdateTestCases(IEnumerable<TestCase> testCases, PathConversionDirection updateDirection)
{
return testCases;
}

TestRunChangedEventArgs IPathConverter.UpdateTestRunChangedEventArgs(TestRunChangedEventArgs testRunChangedArgs, PathConversionDirection updateDirection)
{
return testRunChangedArgs;
}

TestRunCompleteEventArgs IPathConverter.UpdateTestRunCompleteEventArgs(TestRunCompleteEventArgs testRunCompleteEventArgs, PathConversionDirection updateDirection)
{
return testRunCompleteEventArgs;
}

TestRunCriteriaWithSources IPathConverter.UpdateTestRunCriteriaWithSources(TestRunCriteriaWithSources testRunCriteriaWithSources, PathConversionDirection updateDirection)
{
return testRunCriteriaWithSources;
}

TestRunCriteriaWithTests IPathConverter.UpdateTestRunCriteriaWithTests(TestRunCriteriaWithTests testRunCriteriaWithTests, PathConversionDirection updateDirection)
{
return testRunCriteriaWithTests;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;

internal enum PathConversionDirection
{
Receive,
Send
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;
using System.Collections.Generic;
using System.Linq;
using ObjectModel;
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
using System.Collections.ObjectModel;
using Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces;
using System.IO;
nohwnd marked this conversation as resolved.
Show resolved Hide resolved

internal class PathConverter : IPathConverter
{
// The path on this computer to which we deployed the test dll and test runner
private readonly string _deploymentPath = "";
// The path on the remote system where test dll was originally placed, and from which we
// copied it to this system. For vstest.console, which is on the other side of this, the names
// are inverted, it sends us their local path, and thinks about our local path as remote.
private readonly string _originalPath = "";

public PathConverter(string originalPath, string deploymentPath, IFileHelper fileHelper)
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
{
_originalPath = fileHelper.GetFullPath(originalPath).TrimEnd('\\').TrimEnd('/') + Path.DirectorySeparatorChar;
_deploymentPath = fileHelper.GetFullPath(deploymentPath).TrimEnd('\\').TrimEnd('/') + Path.DirectorySeparatorChar;
}

public string UpdatePath(string path, PathConversionDirection updateDirection)
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
{
string find;
string replaceWith;
if (updateDirection == PathConversionDirection.Receive)
{
// Request is incoming, the path that is local to the sender (for us that is "remote" path)
// needs to be replaced with our path
find = _originalPath;
replaceWith = _deploymentPath;
}
else
{
find = _deploymentPath;
replaceWith = _originalPath;
}

var result = path?.Replace(find, replaceWith);
return result;
}

public IEnumerable<string> UpdatePaths(IEnumerable<string> enumerable, PathConversionDirection updateDirection)
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
{
var updatedPaths = enumerable.Select(i => UpdatePath(i, updateDirection)).ToList();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to "execute" the update? I mean, is ToList() required? And if we want to actually do the change, is it worth to have the interface less generic? I don't like patterns where we put everything IEnumerable to then lways cast down to concrete list/array, that's just causing trouble.

Comment applies to most impl in this class.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love to do that this way, but the object contract is public, and the only thing I can do without changing it, is making this work in a terrible side-effectful way.

The select here will work just fine, ToList forces enumeration, but I changed it to use tolist and foreach in all places to make it more obvious that we do something that has side effects.

We don't cast down to anything here, I don't see where this would fail. Care to give an example?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When saying "casting down", I meant that I have often see codebases where you have something like:

var list = new List<int> { 1, 2, 3 };
var x = Update(list);
var y = AnotherUpdate(x);

private IEnumerable<int> Update(IEnumerable<int> en)
    => en.Select(x => x + 1).ToList();

private IEnumerable<int> AnotherUpdate(IEnumerable<int> en)
    => en.Select(x => x + 1).ToList();

So at the end we keep creating list while we already have one at first.

is making this work in a terrible side-effectful way.

Maybe an explicit foreach gives a better clarity on "side-effect"?

I would love to do that this way, but the object contract is public

Not sure to get it, you are adding the IPathConverter interface in this PR, so you could tweak it the way you want. Am I missing something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is "casting" up imho, IEnumerable is higher in the inheritance tree than List. But yeah, what you show is stupid design that is trying to be overly generic, and uses IEnumerable without using the lazy nature of it. I am not trying to do that here, I just use the types that are incoming in the request so I can go from var serialized = Serializer.Serialize(dto) to var serialized = Serializer.Serialize(_pathConverter.Update(dto)).

As for the side-effects, I am updating dtos that have a non-default constructor and non-settable properties, so I can't just create a new one with the updated data. I would love that, but instead I am forced to use side-effects to change the values in the incoming data (in most cases). Without that I would have to change the public api of those dtos, so they allow me to recreate them.

return updatedPaths;
}

public TestCase UpdateTestCase(TestCase testCase, PathConversionDirection updateDirection)
{
testCase.CodeFilePath = UpdatePath(testCase.CodeFilePath, updateDirection);
testCase.Source = UpdatePath(testCase.Source, updateDirection);
return testCase;
}

public IEnumerable<TestCase> UpdateTestCases(IEnumerable<TestCase> testCases, PathConversionDirection updateDirection)
{
var updatedTestCases = testCases.Select(tc => UpdateTestCase(tc, updateDirection)).ToList();

return updatedTestCases;
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
}

public TestRunCompleteEventArgs UpdateTestRunCompleteEventArgs(TestRunCompleteEventArgs testRunCompleteEventArgs, PathConversionDirection updateDirection)
{
UpdateAttachmentSets(testRunCompleteEventArgs.AttachmentSets, updateDirection);
return testRunCompleteEventArgs;
}

public TestRunChangedEventArgs UpdateTestRunChangedEventArgs(TestRunChangedEventArgs testRunChangedArgs, PathConversionDirection updateDirection)
{
UpdateTestResults(testRunChangedArgs.NewTestResults, updateDirection);
UpdateTestCases(testRunChangedArgs.ActiveTests, updateDirection);
return testRunChangedArgs;
}

public Collection<AttachmentSet> UpdateAttachmentSets(Collection<AttachmentSet> attachmentSets, PathConversionDirection updateDirection)
{
attachmentSets.Select(i => UpdateAttachmentSet(i, updateDirection)).ToList();
return attachmentSets;
}

public ICollection<AttachmentSet> UpdateAttachmentSets(ICollection<AttachmentSet> attachmentSets, PathConversionDirection updateDirection)
{
attachmentSets.Select(i => UpdateAttachmentSet(i, updateDirection)).ToList();
return attachmentSets;
}

private AttachmentSet UpdateAttachmentSet(AttachmentSet attachmentSet, PathConversionDirection updateDirection)
{
attachmentSet.Attachments.Select(a => UpdateAttachment(a, updateDirection)).ToList();
return attachmentSet;
}

private UriDataAttachment UpdateAttachment(UriDataAttachment attachment, PathConversionDirection updateDirection)
{
// todo: convert uri?
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
return attachment;
}

private IEnumerable<TestResult> UpdateTestResults(IEnumerable<TestResult> testResults, PathConversionDirection updateDirection)
{
// The incoming collection is IEnumerable, use foreach to make sure we always do the changes,
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
// as opposed to using .Select which will never run unless you ask for results (which totally
// did not happen to me, of course).
foreach (var tr in testResults)
{
tr.Attachments.Select(a => UpdateAttachmentSet(a, updateDirection));
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
UpdateTestCase(tr.TestCase, updateDirection);
}

return testResults;
}

public DiscoveryCriteria UpdateDiscoveryCriteria(DiscoveryCriteria discoveryCriteria, PathConversionDirection updateDirection)
{
discoveryCriteria.Package = UpdatePath(discoveryCriteria.Package, updateDirection);
foreach (var adapter in discoveryCriteria.AdapterSourceMap.ToList())
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
{
var updatedPaths = UpdatePaths(adapter.Value, updateDirection);
discoveryCriteria.AdapterSourceMap[adapter.Key] = updatedPaths;
}
return discoveryCriteria;
}

public TestRunCriteriaWithSources UpdateTestRunCriteriaWithSources(TestRunCriteriaWithSources testRunCriteriaWithSources, PathConversionDirection updateDirection)
{
testRunCriteriaWithSources.AdapterSourceMap.Select(adapter => testRunCriteriaWithSources.AdapterSourceMap[adapter.Key] = UpdatePaths(adapter.Value, updateDirection));
var package = UpdatePath(testRunCriteriaWithSources.Package, updateDirection);
return new TestRunCriteriaWithSources(testRunCriteriaWithSources.AdapterSourceMap, package, testRunCriteriaWithSources.RunSettings, testRunCriteriaWithSources.TestExecutionContext);
}

public TestRunCriteriaWithTests UpdateTestRunCriteriaWithTests(TestRunCriteriaWithTests testRunCriteriaWithTests, PathConversionDirection updateDirection)
{
var tests = UpdateTestCases(testRunCriteriaWithTests.Tests, updateDirection);
var package = UpdatePath(testRunCriteriaWithTests.Package, updateDirection);
return new TestRunCriteriaWithTests(tests, package, testRunCriteriaWithTests.RunSettings, testRunCriteriaWithTests.TestExecutionContext);
}
}
Loading