Missing using instruction in Exclude and Lead

[Orace]

Here the list of not disposed enumerators
Unit test should be added for it (maybe we can do a generic test like NullArgumentTest).

• MoreLinq\Exclude.cs:48
• MoreLinq\Lead.cs:68
  SkipUntil

We should have test for them

[atifaziz]

I somehow doubted the list is long. Let's take your first example of line 70 of Backsert:

MoreLINQ/MoreLinq/Backsert.cs

Line 70 in ce57548

using var e = first.CountDown(index, ValueTuple.Create).GetEnumerator();

How's the enumerator not being disposed? I see a using that has the scope of the method.

maybe we can do a generic test like NullArgumentTest

This is tricky but if you have ideas on how then I am listening. Meanwhile, we have TestingSequence that should be used to help with checking for proper disposal. You can see this is used in tests for Backsert:

MoreLINQ/MoreLinq.Test/BacksertTest.cs

Lines 56 to 63 in ce57548

	public IEnumerable<int> Backsert(int[] seq1, int index, int[] seq2)
	{
	using (var test1 = seq1.AsTestingSequence())
	using (var test2 = seq2.AsTestingSequence())
	{
	return test1.Backsert(test2, index).ToArray();
	}
	}

Now, technically, if you remove the using in the Backsert then the tests should break. What's disturbing is that they don't and so you've highlighted another issue!

As yet another issue, Pairwise is missing the test:

MoreLINQ/MoreLinq.Test/PairwiseTest.cs

Lines 41 to 46 in ce57548

	[Test]
	public void PairwiseWideSourceSequence()
	{
	var result = new[] { "a", "b", "c", "d" }.Pairwise((x, y) => x + y);
	result.AssertSequenceEqual("ab", "bc", "cd");
	}

So you've highlighted new issues by bringing up a potential non-issue. 😄

[Orace]

Indead Reasharper doesn't handle the new inline using.
I will update the list.

[Orace]

SkipUntil has to be fixed and tested too. (Found by a Monkey : #663 )

[atifaziz]

It seems that I have introduced some leaks in 3d44f73 cdc6f76 from PR #621. It seems that there wasn't enough test coverage to make that refactoring confidently. I wrongly assumed that if no tests broke then no leakage was introduced. Either 3d44f73 cdc6f76 should be reverted or it requires a second and closer (eyeball) review. Adding code coverage is becoming crucial going forward.

[Orace]

I'm not sure it's 3d44f73 since it's about use of ??=
Maybe this one: cdc6f76

[Orace]

I read the full diff and find two missing using.
Turn out it was for Exclude and Lead

[atifaziz]

I'm not sure it's 3d44f73 since it's about use of ??=
Maybe this one: cdc6f76

You're right. 🤦‍♂ I copied the wrong commit hash.

[Orace]

SkipUntil dispose properly its enumerator.

[atifaziz]

I read the full diff and find two missing using.
Turn out it was for Exclude and Lead

Hey, thanks a ton for reviewing the diff! ⭐️