morelinq / MoreLINQ

Extensions to LINQ to Objects
https://morelinq.github.io/
Apache License 2.0
3.63k stars 409 forks source link

Revise tests to avoid using `Eumerable.Range` #1044

Open atifaziz opened 7 months ago

atifaziz commented 7 months ago

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:

viceroypenguin commented 7 months ago

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.

atifaziz commented 7 months ago

@viceroypenguin Awesome.

they should all be TestingSequence regardless

PR #475 is addressing that.

viceroypenguin commented 7 months ago

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 commented 7 months ago

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:

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.