Assign an id to each fake, and add it to the fake's string representation

[thomaslevesque]

e.g. Faked IFoo #1

The goal is to make it easier, when debugging, to tell whether the expected Fake is the one that's being exercised by the production code.

The id would be a global counter for all fakes (no need to have a counter per type)

[blairconrad]

We should consider whether this is breaking. We don't make any logical decisions based on the name, but the Fake's ToString will use the value, and some people may be writing tests that examine the result of stringing their Fakes.

[thomaslevesque]

some people may be writing tests that examine the result of stringing their Fakes

Good point. Not sure it's reason enough to not do it, though... I doubt many people are doing this, and there are obvious benefits to having the id.

I guess we could have a flag to restore the original behavior, using runtime config settings and AppContext.TryGetSwitch. It's not something we've done before, but it's pretty common in e.g. ASP.NET Core, when you want to upgrade to a later version but need to opt out of a breaking change. This would allow the few people who actually rely on the fake's string representation to upgrade without having to make too many changes. Or we could make the new behavior opt-in for now, and make it the default in the next major release.

[thomaslevesque]

but it's pretty common in e.g. ASP.NET Core

Actually, now that I think about it, ASP.NET Core uses a different mechanism. But .NET itself uses this (see the examples on the docs page).

It could also be a static property somewhere, but it raises the question of when to initialize it, since there's no obvious entry point in unit tests (a module initializer would be appropriate, but it's only available since C# 9)

[blairconrad]

Oh, man. You got fancy. I was just thinking to do it for 8.0.0 is all.
I don't mind changing the behaviour, as I expect it'll be pretty low impact. I just mind surprising people.

[thomaslevesque]

We need to consider how it should work out with named fakes. Do we write the name and id?
Currently the way we use fake names seems a little inconsistent. We have FakeObjectName and FakeObjectDisplayName properties in FakeManager; sometimes we use one, sometimes we use the other.

[thomaslevesque]

FakeObjectDisplayName uses FakeObjectName as follows:

internal string FakeObjectDisplayName =>
    string.IsNullOrEmpty(this.FakeObjectName)
        ? "Faked " + this.FakeObjectType
        : this.FakeObjectName!;

This is how FakeObjectName and FakeObjectDisplayName are currently used:

• Usage of FakeObjectName

  • AnyCallCallRule.DescribeCallOn (used to describe the call being asserted)

    e.g. "Any call made to the fake object {FakeObjectName}"

  • DefaultFakeObjectCallFormatter (used in the list of calls in call assertion error message)

    e.g. "{call description} on {FakeObjectName}"

  • CallConstraintDescriber (used by ExpressionCallRule.DescribeCallOn to describe the call being asserted)

    e.g. "{call description} on {FakeObjectName}"

• Usage of FakeObjectDisplayName

  • ArgumentFormatter.DefaultFormatter.GetArgumentValueAsString (used to represent the fake as an argument in a call description)

  • FakeManager.ObjectMemberRule.HandleToString (used as the default ToString implementation for fakes)

  • EqualityArgumentConstraint.ToString (used as a fallback if writer.WriteArgumentValue fails)

  • EventAction.WriteDescription (used to describe event subscriptions/unsubscriptions)

    e.g. "Subscription to event '{eventName}' of {FakeObjectDisplayName}"

Here's what I suggest we do:

• FakeObjectDisplayName is updated to append the id, e.g.

  Faked IFoo #123    // if no name is set
  MyNamedFake #123   // if a name is set
  

• In places where we use FakeObjectName directly, append the id, e.g.

  {call description} on #123               // if no name is set
  {call description} on MyNamedFake #123   // if a name is set
  

[blairconrad]

Thanks for collating this, @thomaslevesque. I'm going to need to mull. For example, I'm not sure why we use FakeObjectDisplayName in some places and FakeObjectName in others.

My knee-jerk reaction to (part of) your latest comments is that if Fakes are named, then we use that and forget about a numeric ID. But I'm not set on that…

[thomaslevesque]

My knee-jerk reaction to (part of) your latest comments is that if Fakes are named, then we use that and forget about a numeric ID.

I think it would be useful to include the ID anyway. This could be helpful if the user accidentally created several fakes with the same name (or maybe created a fake that is accidentally reused across multiple tests).

[blairconrad]

I am confused about why we use FakeObjectName in some places and FakeObjectDisplayName in others. I can see neither rhyme nor reason. I am not saying we should unconfuse me first, but maybe we should unconfuse me first.

[thomaslevesque]

Yeah, I was confused too.

FakeObjectDisplayName returns either if the name (if it's set) or "Faked {FakeObjectType}"
In some places, we want to write the name (if it's set) or nothing. If we used FakeObjectDisplayName everywhere, for unnamed fakes we would end up with messages like this:

• Any call made to the fake object Faked IFoo
• IFoo.Bar() on Faked IFoo

Instead of

• Any call made to the fake object
• IFoo.Bar()

"faked object Faked IFoo" is a bit of a mouthful, but it probably wouldn't hurt to be more explicit here.
"IFoo.Bar() on Faked IFoo" is also a bit redundant because the call description already contains "IFoo" (not in all scenarios, though)

We could change the behavior to have "Faked {FakeObjectType}" as the default name (so we would always have a name), and always write the fakes as "{name} #{id}". At least it would be consistent...

[blairconrad]

• IFoo.Bar() on Faked IFoo

instead of

• IFoo.Bar() on Faked IFoo

These are remarkably similar…

"faked object Faked IFoo" is a bit of a mouthful, but it probably wouldn't hurt to be more explicit here.

Yeah. I think seeing "Any call made to Faked IFoo" or similar would be an improvement.

"IFoo.Bar() on Faked IFoo" is also a bit redundant because the call description already contains "IFoo" (not in all scenarios, though)

Agreed. And the issue of "what type should we show on the left of that dot?" came up recently as well. I was wondering whether we couldn't get away with "[Faked IFoo].Bar()" or similar. Not married to the "[]"s, but figure we might need some way to delimit the Fake's name, especially since we have a space in it…

We could change the behavior to have "Faked {FakeObjectType}" as the default name (so we would always have a name), and always write the fakes as "{name} #{id}". At least it would be consistent...

I am interested in exploring this. It may not transform users' experience, but consistency is nice both inside and outside the codebase.

[thomaslevesque]

These are remarkably similar…

Oops! Fixed it

I was wondering whether we couldn't get away with "[Faked IFoo].Bar()" or similar

But that could be misleading. For instance, if the fake also implements another interface that has a Bar method, we need a way to indicate which one it is.

[blairconrad]

Uninformative rather than misleading, I'd say. I see your point, but am not convinced we're reporting "the right one" all the time anyhow.

Sigh. But we're reporting something, which I think is not a lie. You're probably right that it's better to keep a type there.