VerifyAll throws System.InvalidOperationException: 'Nullable object must have a value.'

[aeslinger0]

I'm running a unit test that passed using Moq 4.2.1409.1722 but now fails using 4.10.0 with the exception above. I loaded the symbols for Moq to see if I could figure out what was failing. This is what I saw:

Not sure if related, but note the error when trying to view the expression parameter: "Cannot obtain value of the local variable or argument because it is not available at this instruction pointer, possibly because it has been optimized away."

I went up the call stack to see if expression was being passed in. I can see that expression had a value:

I might be going down the wrong path, but to me it appears like something is causing that expression to disappear inside the call to this.fnCanBeEvaluated(expression) which I am unable to see inside of.

So it might be a bug in Moq or, if it's a problem with my code, then Moq should probably check for the condition that causes the exception and throw an outer exception with a better description.

[stakx]

@aeslinger0 - Thanks for reporting. It would be great if you could post a short, self-contained repro code example or unit test so this can be looked into.

[stakx]

Not sure if related, but note the error when trying to view the expression parameter: "Cannot obtain value of the local variable or argument because it is not available at this instruction pointer, possibly because it has been optimized away."

That's unrelated. It's simply because you've been debugging with a Moq assembly that's been built with the Release configuration.

[aeslinger0]

This should reproduce the problem:

UnitTestProject1.zip

[stakx]

OK, after studying your ZIP file for a while, I think this is what you're dealing with:

[Fact]
public void Test()
{
    int? x = 1;
    var mock = new Mock<Action<int?>>();
    mock.Setup(m => m(x.Value));
    mock.Object(1);
    x = null;
    mock.VerifyAll();
}

VerifyAll will fail because at some point it will try to re-evaluate the original setup expression (which is probably the unexpected & unintuitive bit here!), and because x.Value has become null in the meantime, an exception will be thrown. Had you simply defined your setup as mock.Setup(m => m(1)) (i.e. if you'd inlined the constant) then your test would pass fine.

Will need to take a deeper look, but it does feel like a bug.

[stakx]

On the other hand, what about the following test:

 int? x = 1;
 var mock = new Mock<Action<int?>>();
-mock.Setup(m => m(x.Value));
-mock.Object(1);
 x = null;
-mock.VerifyAll();
+mock.Verify(m => m(x.Value), Times.Never);

This produces the same error, but in this case it's much more obvious why things are going wrong. Should this test succeed or not? I think this one should fail.

[stakx]

After some more pondering I am pretty certain that this is a bug, because there is an internal inconsistency in such a setup: On the one hand, the argument expression gets evaluated eagerly to produce a constant-value argument matcher (used for matching invocations against setups), and on the other hand, the original setup expression gets reevaluated (i.e. lazily) during verification. That doesn't seem to make much sense. Verification should probably also use the eagerly evaluated argument value.

[aeslinger0]

Thanks. Based on your comments, I changed the test from:

var pTransaction = new PTransaction() { PaidByTransactionId = 1, PaidByCartId = 2 };
_lineItemRepositoryMock.Setup(x => x.GetByTransactionId(pTransaction.PaidByTransactionId.Value))
    .Returns(pTransaction);

To:

var pTransaction = new PTransaction() { PaidByTransactionId = 1, PaidByCartId = 2 };
_lineItemRepositoryMock.Setup(x => x.GetByTransactionId(1)).Returns(pTransaction);

The test now passes. Thanks for your help. Good luck with the bug!

[stakx]

Note to self, here's a related unit test; Moq does evaluate some things lazily by design:

https://github.com/moq/moq4/blob/65ce19e75181a479c2d17cd5389f38412dc5c91a/tests/Moq.Tests/ReturnsFixture.cs#L122-L135

It might get quite tricky to correctly draw the line between eager and lazy evaluation of arguments in setup/verify expressions.

[stakx]

The test mentioned in #711 (comment) no longer reproduces this problem in current master, but this small change brings the problem back:

 [Fact]
 public void Test()
 {
     int? x = 1;
     var mock = new Mock<Action<int?>>();
+    mock.Setup(m => m(1));
     mock.Setup(m => m(x.Value));
     mock.Object(1);
     x = null;
     mock.VerifyAll();
 }

The problem occurs when VerifyAll fetches a list of all setups that are still active (i.e. haven't been overridden by later setups). Building that list involves comparing later setups to earlier ones, and this in turn means that setup expressions, including the argument expressions, are compared. This is when re-evaluation happens.

Off the top of my head, I can think of two ways to fix this:

1. When a setup is added that overrides an earlier one, mark that earlier one as overridden right away. Setups are still compared against one another, so this doesn't completely eliminate the possibility of this happening, but it reduces the risk because it is much less likely that state mutations are already occurring during the arrange stage of tests.

2. In InvocationShape, perform partial argument expression evaluation eagerly. Currently, they are evaluated lazily, as can be seen here. In the ctor, only the argument matchers are initialized:

   https://github.com/moq/moq4/blob/2a61db9f2ec17d5d69c2466bf53899b77d5b5e2a/src/Moq/InvocationShape.cs#L45-L46

   But argument expressions are kept as is. They're only evaluated later, in Equals:

   https://github.com/moq/moq4/blob/2a61db9f2ec17d5d69c2466bf53899b77d5b5e2a/src/Moq/InvocationShape.cs#L129-L137

   Do both in the ctor. This should resolve the reported issue.