Skip to content
This repository was archived by the owner on Jan 11, 2024. It is now read-only.

Add XUnitFilter wrapper to exclude Methods, Classes and Namespaces#2035

Merged
A-And merged 3 commits into
dotnet:masterfrom
A-And:XUnitFilterExperiment
May 10, 2018
Merged

Add XUnitFilter wrapper to exclude Methods, Classes and Namespaces#2035
A-And merged 3 commits into
dotnet:masterfrom
A-And:XUnitFilterExperiment

Conversation

@A-And
Copy link
Copy Markdown

@A-And A-And commented May 9, 2018

Workaround until xunit/xunit#1701 is resolved.
This allows methods/namespaces and classes to be skipped via command line switches.
This doesn't support regex or wildcards, so the names passed need to include a fully-qualified namespace.

@weshaggard, could you take a look?

cc @sergiy-k

@A-And A-And requested a review from weshaggard May 9, 2018 22:58
@weshaggard
Copy link
Copy Markdown
Member

Assuming it works this seems reasonable to me. While this looks like a step in the right direction I thought you guys wanted to feed it a file with things to exclude?

cc @BruceForstall

@A-And
Copy link
Copy Markdown
Author

A-And commented May 9, 2018

@weshaggard That's the goal. I wanted to get an opinion on this while we settle on the best way to consume it across repos.

{
// Accept formats of both -Qualifier Name and -Qualifier:Name
if (delimiters == null)
delimiters = new char[] { ' ', ':' };

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Copy link
Copy Markdown
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

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

Looks reasonable.

@A-And A-And merged commit 4918a52 into dotnet:master May 10, 2018
@A-And A-And deleted the XUnitFilterExperiment branch May 10, 2018 21:59
@BruceForstall
Copy link
Copy Markdown

@A-And I don't know how to track this merge back to the tools used by the dotnet/coreclr repo. Is it being used there now? If not, when?

btw, thanks for this work!

@A-And
Copy link
Copy Markdown
Author

A-And commented May 16, 2018

@BruceForstall
No issues!

The XUnit Console Runner is published at https://dotnet.myget.org/feed/dotnet-buildtools/package/nuget/xunit.console.netcore .

In CoreCLR the package dependency is defined in https://github.com/dotnet/coreclr/blob/master/dependencies.props under <XunitConsoleNetcorePackageVersion> . The current dependency is on an old runner (it doesn't change much), so these changes are not currently being used. Rolling the runner forward to version 2.2.0-preview1-02815-01 which has the newest changes should make the flags available in CoreCLR.

The big caveat here is that CoreFX uses an old runner (prior to these changes). To run the tests under CoreCLR we clone the repo, build and run it, so to use this we need both the latest commit and the runner dependency in CoreFX to be updated,

@BruceForstall
Copy link
Copy Markdown

Do you plan to update the dependency specification in either corefx or coreclr to pick up your changes?

@A-And
Copy link
Copy Markdown
Author

A-And commented May 16, 2018

@BruceForstall Yes, once we enable and use the for CI in CoreRT and the runner is behaving stably (as it should, this is a pretty simple wrapper that shouldn't interfere with anything) then I'm planning on updating the versions on both.

This is probably unnecessarily cautious, but the runner is used by Helix for daily test builds.

@ViktorHofer
Copy link
Copy Markdown
Member

@A-And have we already used that feature somewhere in our repos? If we would switch to the last recent version of the official runner which doesn't contain this feature, would that be an issue?

@A-And
Copy link
Copy Markdown
Author

A-And commented Jun 1, 2018

@ViktorHofer This is used in CoreRT and will be used in CoreCLR soon.

If we would switch to the last recent version of the official runner

What do you mean by the official runner? Is CoreFX moving away from using the xunit.console.netcore wrapper?

@BruceForstall
Copy link
Copy Markdown

@A-And @ViktorHofer This change added support for xunit namespace/class/function exclusions, which have subsequently been added to the XUnit mainline which has been picked up by CoreFX/CoreCLR. However, it also added support for response files. As far as I can tell, the response file parsing and handling code has NOT been added to XUnit mainline. And, subsequently, with #2107, all the xunit code was deleted from buildtools.

So have we lost all the response file code?

AFAIK, we're continuing to use the xunit.console.netcore.exe that includes this support.

So what is the plan going forward? Should we intend to get rid of this xunit.console.netcore.exe dependency and roll forward to the "live" xunit?

cc @echesakovMSFT @RussKeldorph

@ViktorHofer
Copy link
Copy Markdown
Member

ViktorHofer commented Nov 15, 2018

We are going to fork the xunit.console runner from xunit/xunit into arcade as there are multiple issues with the upstream one. Thanks for reaching out and reminding me that this changes didn't make it into upstream. I'll include it in the fork. I'll probably have time for it over the weekend. Does that work for you?

EDIT: Another reason why we are doing this now is because we were informed that the official runner isn't supported anymore and will be removed in a subsequent release.

@BruceForstall
Copy link
Copy Markdown

Yes.

I presume you will also be picking up @echesakovMSFT 's xunit/xunit#1846?

Once forked, I presume you'll build and update the myget feed, and then update corefx/coreclr to use the new package?

I think we're still seeing https://github.com/dotnet/coreclr/issues/19890 sometimes, which presumably will also require some kind of fix.

@ViktorHofer
Copy link
Copy Markdown
Member

If we still use that code path with our forked runner, yes. I need to take a look if we want to move everything over as it is.

Once forked, I presume you'll build and update the myget feed, and then update corefx/coreclr to use the new package?

Yes and yes for corefx. For coreclr I hope that some of you can help me. looking at @A-And ;)

@A-And
Copy link
Copy Markdown
Author

A-And commented Nov 15, 2018

Glad to see a decision was reached on how to handle the xunit runner. Happy to help!

cc @josalem

@BruceForstall
Copy link
Copy Markdown

@ViktorHofer Is there an issue tracking the work to fork xunit.console runner into arcade, and get it used by corefx/coreclr? What's the status?

cc @echesakovMSFT

@ViktorHofer
Copy link
Copy Markdown
Member

@josalem is doing the initial port and I'm going to help him with that. I don't think we have started yet.

@BruceForstall
Copy link
Copy Markdown

@ViktorHofer @josalem Is there any update on the plan to fork xunit.console? Is there an issue that tracks the work (or can you create one)?

https://github.com/dotnet/coreclr/issues/19890 is still quite troublesome, and it would be nice to have someplace where we can attempt to fix it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants