DelayedConstraint doesn't appear to poll properties of objects

Bug #890129 reported by GT
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
NUnit Framework
Fix Released
High
Charlie Poole
NUnit V2
Fix Released
Medium
Charlie Poole

Bug Description

Using 2.5.10.11092.

I've just been exploring the new (to me) fluent/constraints syntax, specifically the After() constraint and am very impressed.
I've already learnt not to pass by value (!), however, I thought that a pass-by-reference should be ok.

For example:
         var worker = new BackgroundWorker();
         var list = new List<int>();
         worker.RunWorkerCompleted += delegate { list.Add(1); };
         worker.DoWork += delegate { Thread.Sleep(1); };
         worker.RunWorkerAsync();
         Assert.That(list, Has.Count.EqualTo(1).After(5000, 100));

The documentation states that its "intended use is with delegates and references", so I'd expect the 'list' object reference to be polled, and after about 1 second, for the constraint to pass.
However, I have to wait for the full 5 seconds before the test succeeds.

The obvious work around is to convert the 'list' object to a delegate:
         Assert.That(() => list, Has.Count.EqualTo(1).After(5000, 100));

This works fine, and still look ok (to me).

However, I wondered if this is deliberate or unavoidable behaviour or just a bug...

Revision history for this message
Charlie Poole (charlie.poole) wrote :

When the DelayedConstraint is used with an object (value or reference) it simply waits the specified amount of time before evaluating the base constraint and no polling takes place. When it is used with a reference to an object or with a delegate, polling is used.

So, the following should work as expected:
  Assert.That(ref list, Has.Count.EqualTo(1).After(5000, 100));

In the example you give, polling could effectively have been used, since the actual value tested is a property of the object and not the object itself. In other cases, this would not work, since the actual value argument is evaluated only once, before calling Assert.That(). A simple example...

   var worker = new BackgroundWorker();
   int x = 0;
   worker.RunWorkerCompleted += delegate { x = 1; }
   worker.DoWork += delegate { Thread.Sleep(1); }
   worker.RunWorkerAsync();
   Assert.That(x, Is.EqualTo(1).After(5000);

The value of x used in the test is zero because x was evaluated before calling Assert.That().

I'll give this a little more thought to determine whether NUnit can make a runtime distinction between those arguments where polling might make sense and those where it would not. Clearly, we can't make the distinction syntactically.

Charlie

Revision history for this message
Simone Busoli (simone.busoli) wrote : Re: [Nunit-core] [Bug 890129] Re: DelayedConstraint doesn't appear to poll properties of objects
Download full text (3.2 KiB)

Interesting one, perhaps inspecting the type of the constraint we could
figure out whether we should poll or not.
On Nov 19, 2011 10:28 AM, "Charlie Poole" <email address hidden> wrote:

> When the DelayedConstraint is used with an object (value or reference)
> it simply waits the specified amount of time before evaluating the base
> constraint and no polling takes place. When it is used with a reference
> to an object or with a delegate, polling is used.
>
> So, the following should work as expected:
> Assert.That(ref list, Has.Count.EqualTo(1).After(5000, 100));
>
> In the example you give, polling could effectively have been used, since
> the actual value tested is a property of the object and not the object
> itself. In other cases, this would not work, since the actual value
> argument is evaluated only once, before calling Assert.That(). A simple
> example...
>
> var worker = new BackgroundWorker();
> int x = 0;
> worker.RunWorkerCompleted += delegate { x = 1; }
> worker.DoWork += delegate { Thread.Sleep(1); }
> worker.RunWorkerAsync();
> Assert.That(x, Is.EqualTo(1).After(5000);
>
> The value of x used in the test is zero because x was evaluated before
> calling Assert.That().
>
> I'll give this a little more thought to determine whether NUnit can make
> a runtime distinction between those arguments where polling might make
> sense and those where it would not. Clearly, we can't make the
> distinction syntactically.
>
> Charlie
>
> --
> You received this bug notification because you are a member of NUnit
> Developers, which is subscribed to NUnit V2.
> https://bugs.launchpad.net/bugs/890129
>
> Title:
> DelayedConstraint doesn't appear to poll properties of objects
>
> Status in NUnit V2 Test Framework:
> New
>
> Bug description:
> Using 2.5.10.11092.
>
> I've just been exploring the new (to me) fluent/constraints syntax,
> specifically the After() constraint and am very impressed.
> I've already learnt not to pass by value (!), however, I thought that a
> pass-by-reference should be ok.
>
> For example:
> var worker = new BackgroundWorker();
> var list = new List<int>();
> worker.RunWorkerCompleted += delegate { list.Add(1); };
> worker.DoWork += delegate { Thread.Sleep(1); };
> worker.RunWorkerAsync();
> Assert.That(list, Has.Count.EqualTo(1).After(5000, 100));
>
> The documentation states that its "intended use is with delegates and
> references", so I'd expect the 'list' object reference to be polled, and
> after about 1 second, for the constraint to pass.
> However, I have to wait for the full 5 seconds before the test succeeds.
>
> The obvious work around is to convert the 'list' object to a delegate:
> Assert.That(() => list, Has.Count.EqualTo(1).After(5000, 100));
>
> This works fine, and still look ok (to me).
>
> However, I wondered if this is deliberate or unavoidable behaviour or
> just a bug...
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/nunitv2/+bug/890129/+subscriptions
>
> _______________________________________________
> Mailing list: https://launchpad.net/~nunit-core
> Post to : nunit-cor...

Read more...

Revision history for this message
Charlie Poole (charlie.poole) wrote :

After giving it some thought, I decided

1) It's intuitive to expect the provided example to work.

2) In those cases that will not work due to prior evaluation of the argument, polling will not change the outcome but simply cause a bit of overhead.

3) It's simplest to poll in all cases where a polling interval is provided (i.e. do what the user said to do).

4) In future, it might be useful to give a warning when the actual value is a simple type.

Charlie

Changed in nunitv2:
status: New → In Progress
importance: Undecided → Medium
assignee: nobody → Charlie Poole (charlie.poole)
milestone: none → 2.6.0b2
Changed in nunitv2:
status: In Progress → Fix Committed
Changed in nunit-3.0:
status: New → Triaged
importance: Undecided → High
assignee: nobody → Charlie Poole (charlie.poole)
Changed in nunit-3.0:
milestone: none → 2.9.6
Changed in nunitv2:
status: Fix Committed → Fix Released
Changed in nunit-3.0:
status: Triaged → In Progress
Changed in nunit-3.0:
status: In Progress → Fix Committed
Changed in nunit-3.0:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.