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

Allow to have stable theory case name when argument values are dynamic #1386

Closed
zvirja opened this issue Aug 2, 2017 · 5 comments
Closed

Comments

@zvirja
Copy link

zvirja commented Aug 2, 2017

Background

There is the AutoFixture project which aim is to help with test data generation. We have an integration of AutoFixture and xUnit that provides custom Data attributes (like this) one. The implementation is quite straightforward - we scan parameter types and return auto-generated data for them.

The issue is that we generate our data randomly, so each time the test is executed, we provide the different input data. Obviously, with such scenario it's impossible to perform the discovery, so we disabled it via custom DataDiscoverer. However, there are still problems with third-party tools. For instance, NCrunch doesn't want to support such theories entirely. Also there are constant issues with ReSharper (e.g. this or this) which are long running, often affect performance and hurts a lot.

Feature request

I want to have a way to change DisplayName for the theory cases, produced by the data attributes. Currently, xunit generates display name using resolved argument values. I'd like to have an opportunity to override that display name (e.g. to make it stable in case of AutoFixture).

Consider the following sample.
Method:

[Theory, AutoData]
public void SomeTheory(int arg1, string arg2) {}

Current display name:

...SomeTheory(arg1: 33, arg2: "arg2974421d1-52b3-4f11-80ff-81003407d1ed")

Desired stable display name (a sample):

...SomeTheory(arg1: auto<int>, arg2: auto<string>)

I want to be able to provide static placeholders for the dynamically generated members.

Also, AutoFixture has the InlineAutoData attribute that is like InlineData - some arguments are fixed, but rest are auto-generated. For them I'd like to have the output like:

...SomeTheory(arg1: 42, arg2: "fixed string", arg3: auto<int>, arg4 auto<string>)

I want to have a control over the display name and how the particular arguments are being rendered.

I do understand that with such scenario user will not be able to see the real theory case data - but I want to have a choice, so user could decide whether he needs such a feature or not.

Potential solutions

Way 1 - Add an interface for display name formatting

We could create an additional interface the test discoverer could implement. Later when we render the test name, we will check whether the discoverer can perform its own formatting. If can - ask it about that, if not - fallback to the existing behavior.

Example:

public interface ITheoryCaseDisplayNameFormatter
{
    string GetDisplayNameWithArguments(IAttributeInfo dataAttribute, IMethodInfo method, string baseDisplayName, object[] arguments, ITypeInfo[] genericTypes);
}

Change code here to try to cast current data discoverer to ITestCaseNameFormatter and use its method instead.

Way 2 - Like way 1, but inherit the IDataDiscoverer

We could define a new kind of data discoverer than have an extra method. Interface will look like this:

public interface IDataDiscovererWithCustomDisplayName: IDataDiscoverer
{
    string GetDisplayNameWithArguments(IAttributeInfo dataAttribute, IMethodInfo method, string baseDisplayName, object[] arguments, ITypeInfo[] genericTypes);
}

The rest is like in the way 1 - we use this method if needed.

Way 3 - Add an interface to return different args for the display

One more way to solve the issue might be to return custom arguments for display name. The idea is that I can return my custom objects for display, so they will be rendered differently (because I'll override the ToString() method).

Interface:

public interface ITheoryCustomDisplayNameArguments
{
    object[] GetDisplayArguments(IAttributeInfo dataAttribute, IMethodInfo method, object[] arguments);
}

This way will allow to use different arguments

This way will require the least efforts on AutoFixture side and will require a little support, however it looks a bit ad-hoc and might be useful for the AutoFixture only.

Discussion

I don't mind to make a PR to xunit and implement this myself, but previously I want to agree on approach. Therefore:

  1. Is that fine for xunit to be extended with such kind of functionality?
  2. Is that realistic to release this as a part of xunit 2.3?
  3. Do you see more elegant ways to achieve the desired behavior? If not, which approach from the suggested you would suggest to go?
  4. Do you see some pitfalls with the suggested feature? Of course, it should be used with the caution to e.g. not produce duplicated theory names.

If you have some other questions - please let me know.

P.S. This issue is likely a reopen of #649, but here I describe what exactly I want to achieve.

@zvirja
Copy link
Author

zvirja commented Aug 21, 2017

@bradwilson Could you please take a look on this or invite somebody else who could help? I know proposal contains a lot of text, but can't see how to make it shorter. AutoFixture is quite popular library and community will win from this change.

@bradwilson
Copy link
Member

Unfortunately, this is not an easy thing to change. It's buried inside TestMethodTestCase in a way that's currently not easy to extend. At the time these display names are being generated, we have long been distanced from the data attributes in question, not the least of which because this is done during test case deserialization, which may be happening at an entirely different time (and in a different process) from the original discovery.

It may be possible to thread that information through but it would be an extensive undertaking and would be a breaking change to several extensibility points. In essence, the change would need to be that TestMethodTestCase is given the formatted arguments ahead of time, which means that this information would flow through probably a dozen classes or more (breaking everything is touches, since pretty much everything along the way is a potential extensibility point).

My belief is that the simplest fix w/ the best usability (without breaking the world) would be to turn off theory data pre-enumeration for AutoData.

@zvirja
Copy link
Author

zvirja commented Aug 24, 2017

@bradwilson Thank you a lot for your time and reply! I've checked that in more detail and it appeared that actually xUnit and VS runner performs fine with theories with random data, as soon as data attributes report that they cannot be pre-discovered.

My belief is that the simplest fix w/ the best usability (without breaking the world) would be to turn off theory data pre-enumeration for AutoData.

Already done and VS test runner seems to perform fine with that!

I mostly created this issue because NCrunch doesn't support AutoFixture blaming the theory cases with random data. Given that xUnit seems to support such theories, I'm currently discussing with @remcomulder (NCrunch dev) whether they see any limitations from their side. If that so - we'll report them there.

In the meanwhile please don't close this issue in case we'll find xUnit API is not enough currently.

@zvirja
Copy link
Author

zvirja commented Aug 26, 2017

@bradwilson During the recent investigations NCrunch team (@remcomulder) confirmed that actually everything goes fine with AutoFixture and its dynamic theories. It appeared that xUnit is ready for that by means of non-discoverable theories.

xUnit is awesome as always 😉 🥇

Closing this one as no actions is required actually. Brad, thanks for your time!

@Mik4sa
Copy link

Mik4sa commented Mar 30, 2022

Sorry for asking this in a closed issue, but I just stumbled about this issue. I wonder if this is still a problem.
I would love to have this working.
@zvirja @bradwilson Any news from your side?

Thanks in advance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants