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

TreeNodeFilterCommandLineOptionsProvider seems to be using the wrong Version #4294

Open
Youssef1313 opened this issue Dec 9, 2024 · 5 comments
Assignees
Labels
Area: MTP Belongs to the Microsoft.Testing.Platform core library

Comments

@Youssef1313
Copy link
Member

/// <inheritdoc />
public string Uid { get; } = extension.Uid;
/// <inheritdoc />
public string Version { get; } = AppVersion.DefaultSemVer;
/// <inheritdoc />
public string DisplayName { get; } = extension.DisplayName;
/// <inheritdoc />
public string Description { get; } = extension.Description;

To me that's using the platform version but it should use the version of whatever extension being passed.

@Youssef1313 Youssef1313 added the Area: MTP Belongs to the Microsoft.Testing.Platform core library label Dec 9, 2024
@Evangelink
Copy link
Member

I am actually wondering if we should keep the platform version as we own the implementation. This means we would need to fix what we have done for the bridge extensions.

@Evangelink Evangelink self-assigned this Jan 28, 2025
@Evangelink Evangelink added this to the MSTest 3.8 / Platform 1.6 milestone Jan 28, 2025
@MarcoRossignoli
Copy link
Contributor

We own all the logic so I think that we should keep our version, in case of issues we need to know our version not the user version.

@Youssef1313
Copy link
Member Author

In that case, Uid and Description should be ours as well. Currently it's a mix.

And also if we are ending up not needing extension to be given by MTP users, then this parameter will be unused:

public static void AddTreeNodeFilterService(this ITestApplicationBuilder testApplicationBuilder, IExtension extension)
=> testApplicationBuilder.CommandLine.AddProvider(() => new TreeNodeFilterCommandLineOptionsProvider(extension));

Then we may need to take the break in a next minor release (given it's marked experimental already), and remove the unneeded parameter

@MarcoRossignoli
Copy link
Contributor

It's fine to me keep our naming everywhere, are good for troubleshooting.

@Evangelink
Copy link
Member

We are grouping command lines by extensions under info. I am wondering if we should declare them as platform or keep them under the extension so they are still part of the framework.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: MTP Belongs to the Microsoft.Testing.Platform core library
Projects
None yet
Development

No branches or pull requests

3 participants