Assert.That() doesn't like properties with scoped setters

[Charlie Poole]

Public Class SomeClass
    Public Property BrokenProp() As String
        Get
            Return String.Empty
        End Get
        Private Set(ByVal value As String)
        End Set
    End Property
End Class

<TestFixture()> Public Class clsTestTask
    <Test()> Public Sub TestXyz()
        Dim obj As New SomeClass()
        Expect(obj.BrokenProp, EqualTo(String.Empty))
    End Sub
End Class

The above code produces a compiler error:
Error 1 'Set' accessor of property 'BrokenProp' is not accessible.

If the scoping wasn't applied to the setter on the property or the property
was ReadOnly, the compile error goes away.

This is a .NET 2.0 assembly, compiled with the Visual Studio 2008 SP1.

NUnit 2.4.7 and prior did not experience this issue.

BTW, I've also started a Google Group posting for this issue:
http://groups.google.com/group/nunit-discuss/browse_thread/thread/edb56db3d3e60195

[From SF:2792355]

SF Comment:
More info... the equivalent scoped setter in C# does not have this problem.
It arises solely with VB.

[Chad Kittel (ckittel)]

For what it's worth, I opened up a StackOverflow question that is directly related to this issue. http://stackoverflow.com/questions/2128283/bug-in-vb-compiler-and-or-intellisense-in-both-c-and-vb-wrt-out-of-scope-propert

It appears that the VB.NET compiler simply handles the case of fn(Object) and fn(ref T)(T) differently than C#. It's an unfortunate thing. Technically, one could argue that the C# compiler should change to match VB's behavior in this case. I think it's pretty crappy that both compliers choose a different function to call in this case.

So, if this isn't something that is going to be fixed on the compiler's end, we would want to shift our focus on to NUnit and how we can work around this. From a user's point of view, one workaround would be to call the Asset.That in the following way... Assert.That((obj.BrokenProp), EqualTo(String.Empty)), forcing the obj.BrokenProp to be passed in ByVal. Which, works fine, leading me to the question of... why do the templated Assert.That<T>(ref T actual, ...) functions, need the actual value to be by reference anyhow? Maybe I didn't dig into the 2.5.3 NUnit code deep enough to find out why, but i didn't see anything that jumped out at me that said those overloads (the the Match(...) functions that they call) need to be reference parameters. I was able to strip all of the 'ref's out of these functions, the code compiled and my existing test suite ran just fine. I'm guessing there is a valid reason for the ref parameter there and my test cases just never hit that reason. But if there isn't, then that would be one way to close this issue.

[Charlie Poole (charlie.poole)]

I'm revisiting this old bug to see if anything has changed and to try to resolve it one way or the other for the final NUnit 2.x release. At the same time, I'll answer the last (very old) post, which I seem to have missed when it appeared.

Recapping the bug status: VB generates code to call the overloaded Assert.That<T>(ref T actual, ...) methods whenever the first argument is a property. I have verified that this happens in all cases of a property argument. When the setter is private, the VB compiler generates an error. In fact, as you will see below, VB seems to use the ref overloads on other occasions as well.

Workarounds:
1. Put the actual value into a temporary
2. Putting the argument in parentheses, i.e. (arg.BrokenProp)

Surprisingly, both of these workarounds still call the ref T overloads, but the compiler error is avoided and the test succeeds.

Chad asks why we need the ref T overloads. They exist for use with the DelayedConstraint, invoked by using the .After. syntax. For example:
     Assert.That(ref flag, Is.True.After(3).Seconds)

I see three ways to resolve this:

1. Just document the problem and workarounds. That's what I'll do if there is no further response here.

2. Use a separate method for the ref calls. For example,
       Assert.ByRef(flag, Is.True.After(3).Seconds)
    The name might be something different, of course. I don't like this much, since it's a breaking change
    and will impact C# developers as well as VB developers. If we did it, we would wait until NUnit 3.0 in any case.

3. Provide a separate method for VB folks who are encountering this problem. For example:
       Assert.ByVal(obj.BrokenProp, Is.EqualTo(string.Empty))
    This is my favorite, if we fix it at all, since it only affects those suffering the problem.

Comments and other suggestions are welcome.

Charlie

[Chad Kittel (ckittel)]

Thank you for not abandoning it. I'm no longer affected by this personally, because I have moved on to a different team, using a different technology (C#). That said, the team I left is still constrained by this limitation of course.

I too really like Option #3, even if it's a pollution an otherwise clean/straight-forward overload chain, it would allow the team to move forward and no longer be locked into the last supported version. It kicks the can down the road without ignoring the problem within the framework. It's a trap door for those that need it, and can be ignored by those that don't. The IntelliSense comment should probably explain that this isn't method isn't designed for "typical" usage.

[Charlie Poole (charlie.poole)]

I added Assert.ByVal methods equivalent to Assert.That, but without the offending overload. Note that there is no equivalent in the AssertionHelper syntax (Expect) so you have to use Assert.ByVal if the problem applies.

[Charlie Poole (charlie.poole)]

At this time, the fix in NUnit 3.0 matches that in NUnit 2.6 - that is, we support a new method Assert.ByVal(...) for use by VB developers encountering the problem. It's possible that the API will change in the final release.