Self-contained item in array causes stack overflow

[CharliePoole]

See https://bugs.launchpad.net/nunit-3.0/+bug/1051847

Fix needs to be checked to see if it applies to nunit 3.0 and, if so, ported.

[elbaloo]

I was able to reproduce this issue in master (4b6c38d)

[CharliePoole]

Thanks for letting us know. @simoneb can you port your fix?

[simoneb]

I've tried to plainly port the code but it does not work. Equality comparisons have changed quite a bit in 3.0 and the same changes done to NUnitEqualityComparer don't work because it does no longer seem to be the entry point for all comparisons. Constraints operating on multiple items now do the iteration of the enumerables themselves, therefore the NUnitEqualityComparer instance that they rely on cannot keep track of items for recursion properly. I'm not sure why this was done this way in v3, but it makes porting this functionality more work and less clean because the recursion logic needs to be extracted from the NUnitEqualityComparer into clients, to varying degrees depending on how we want to implement it.

[CharliePoole]

I will take a look to see how and why this was done... it's not a high
priority so let's not worry about it for the moment.

On Fri, Mar 14, 2014 at 6:34 AM, simoneb notifications@github.com wrote:

I've tried to plainly port the code but it does not work. Equality
comparisons have changed quite a bit in 3.0 and the same changes done to
NUnitEqualityComparer don't work because it does no longer seem to be the
entry point for all comparisons. Constraints operating on multiple items
now do the iteration of the enumerables themselves, therefore the
NUnitEqualityComparer instance that they rely on cannot keep track of
items for recursion properly. I'm not sure why this was done this way in
v3, but it makes porting this functionality more work and less clean
because the recursion logic needs to be extracted from the
NUnitEqualityComparer into clients, to varying degrees depending on how
we want to implement it.

Reply to this email directly or view it on GitHubhttps://github.com//issues/52#issuecomment-37646979
.

[constructor-igor]

NUnit v3Beta4Console crashed on test:

    [TestFixture]
    public class NUnitIssue52
    {
        class SelfContainer : IEnumerable
        {
            public IEnumerator GetEnumerator() { yield return this; }
        }
        [Test]
        public void SelfContainedItemFoundInArray()
        {
            var item = new SelfContainer();
            var items = new SelfContainer[] { new SelfContainer(), item };

            // work around
            Assert.True(((ICollection<SelfContainer>)items).Contains(item));

            // causes StackOverflowException
            Assert.Contains(item, items);
            Console.WriteLine("test completed");
        }
    }

[CharliePoole]

Does a timeout prevent the crash and report the test as failing?

[constructor-igor]

at least 4 seconds (on my computer) between message "... to StackOverflowException" and crash dialog.

[CharliePoole]

There's no doubt it's a bug. Did you come across it in your normal work or just by trying to make it happen following this issue report? In other words, is there a reason to push up the priority?

[constructor-igor]

I found the crash during the issue's investigation. So, I don't see a reason to push up the priority.
On the other hand, if nunit console catches StackOverflowException exception, why it cannot process it without crash?