MoreLINQ icon indicating copy to clipboard operation
MoreLINQ copied to clipboard

Revise tests to avoid using `Eumerable.Range`

Open atifaziz opened this issue 1 year ago • 4 comments

Starting with .NET 8, Eumerable.Range returns an instance of a private RangeIterator that implements IList<T> (see also dotnet/runtime#88249). A lot of the unit tests potentially went with the assumption that Eumerable.Range returns a pure sequence. There are some operators in MoreLINQ that are optimised for sources that are collections and/or lists so tests using Eumerable.Range may be testing those optimised paths rather than the actual intended path iterating the source as an IEnumerable<>. Those tests need to be revised to avoid Enumerable.Range altogether.

Potential candidates that need revision:

atifaziz avatar Nov 16 '23 18:11 atifaziz

I'll create the branch, that way we can each work on it. Honestly, they should all be TestingSequence regardless, and then it doesn't matter.

viceroypenguin avatar Nov 16 '23 18:11 viceroypenguin

@viceroypenguin Awesome.

they should all be TestingSequence regardless

PR #475 is addressing that.

atifaziz avatar Nov 16 '23 18:11 atifaziz

Ah, I remember this one. I used it as a foundation for some of the changes I did in SuperLinq to handle this consistency. I'll pull in the commits here and offer some additional improvements for you to decide on. I'll push something shortly.

viceroypenguin avatar Nov 16 '23 18:11 viceroypenguin

Created the first draft in e3c5554. This should be a good foundation to start editing the other tests to match.

After review, I didn't see that much in #475 that was usable here, though there may be some overlap you see that I don't.

For the first commit, I did the following:

  • Add .ToTestData() extension method
    • This simplifies the creation of different testing sequences
    • Actually uses TestingSequence for SourceKind.Sequence instead of a simple sequence
  • Update all AggregateRight tests to use TestingSequence, either directly or via .ToTestData().

One open problem that you can probably solve quicker than me: The various tests that use TestCaseSource are not folding up into child properties of their method test. The existing test case methods don't exhibit the same behavior, so I'm not sure if the difference is because they're void methods, or if there is another factor. I don't use nUnit myself, having switched to the much simpler xUnit, so I don't have the background to know this without further research.

viceroypenguin avatar Nov 16 '23 19:11 viceroypenguin