Gui.cs: MainLoop: Calling AddIdle multiple times with the same function will cause indeterminate behavior

[tig]

I'm writing unit tests for MainLoop. The exercise is clearly illustrating the advantages of test-driven-development, esp for APIs like this. Tons of poor design decisions and resulting indeterminate behavior being exposed. This is one of the worst and should be called out:

[Fact]
public void AddTwice_Function_CalledOnce ()
{
    var driver = new FakeDriver ();
    var ml = new MainLoop (driver);

    var functionCalled = 0;
    Func<bool> fn = () => {
        functionCalled++;
        return true;
    };

    functionCalled = 0;
    ml.AddIdle (fn);
    ml.AddIdle (fn);
    ml.MainIteration ();
    // BUGBUG: Since Remove relies on fn equality AddIdle should not allow adding more than once
    // Once fixed, this 2 should change to a 1.
    Assert.Equal (2, functionCalled);
}

One potential fix is for AddIdle() to call List.Exists before adding and to not add the fn if it is already in the list.

[BDisp]

But it's not possible to add twice the same function? Imagine if two objects call to same function, that's wrong? Maybe I'm wrong and if so I'm sorry.

[tig]

If you call AddIdle() with the same fn twice, that fn gets called twice.

That is a bug. The docs for AddIdle are pretty clear the return value is a token, but in reality it just returns the passed fn: Executes the specified @idleHandler on the idle loop. The return value is a token to remove it.

[tig]

I'm completely wrong about this. This proves it:

[Fact]
public void AddTwice_Function_CalledTwice()
{
    var driver = new FakeDriver ();
    var ml = new MainLoop (driver);

    var functionCalled = 0;
    Func<bool> fn = () => {
        functionCalled++;
        return true;
    };

    functionCalled = 0;
    ml.AddIdle (fn);
    ml.AddIdle (fn);
    ml.MainIteration ();
    Assert.Equal (2, functionCalled);

    functionCalled = 0;
    ml.RemoveIdle (fn);
    ml.MainIteration ();
    Assert.Equal (1, functionCalled);

    functionCalled = 0;
    ml.RemoveIdle (fn);
    ml.MainIteration ();
    Assert.Equal (0, functionCalled);
}

and

[Fact]
public void AddIdle_Twice_Returns_False_Called_Twice()
{
    var driver = new FakeDriver ();
    var ml = new MainLoop (driver);

    var functionCalled = 0;
    Func<bool> fn1 = () => {
        functionCalled++;
        return false;
    };

    // Force stop if 10 iterations
    var stopCount = 0;
    Func<bool> fnStop = () => {
        stopCount++;
        if (stopCount == 10) {
            ml.Stop ();
        }
        return true;
    };

    ml.AddIdle (fnStop);
    ml.AddIdle (fn1);
    ml.AddIdle (fn1);
    ml.Run ();
    ml.RemoveIdle (fnStop);
    ml.RemoveIdle (fn1);
    ml.RemoveIdle (fn1);

    Assert.Equal (2, functionCalled);
}

Nevermind.