Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add UsingTestingSequence to enforce simpler & correct usage #475

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

atifaziz
Copy link
Member

@atifaziz atifaziz commented Apr 27, 2018

This PR obsoletes the use of AsTestingSequence in favour of the new UsingTestingSequence. This is done to enforces simpler & more correct usage.

TestingSequence is primarily designed to help test two things in an operator:

  1. It doesn't enumerate the sequence more than once
  2. It disposes the sequence

However, most uses seemed to be exercising the first but not the second. The latter was probably often skipped/missed because it required more non-obvious setup; an explicit using block (non-obvious usage) as well as eager evaluation before assertion of results. Take the following example:

var result = sequence.AsTestingSequence().Rank();

This will only ensure that Rank doesn't iterate sequence more than once when it's iterated during testing. To also test that it disposes the source iterator when done, the above has to be written as:

IEnumerablt<int> result;
using (var ts = sequence.AsTestingSequence())
    result = ts.Rank().ToArray();

With UsingTestingSequence, you just do:

var result = sequence.UsingTestingSequence(ts => ts.Rank());

TestingSequence Uses Needing Review

  • CompareCountTest
  • CountDownTest
  • EndsWithTest
  • EquiZipTest
  • FlattenTest
  • FoldTest
  • InterleaveTest
  • MoveTest
  • NestedLoopTest
  • RankTest
  • SortedMergeTest
  • StartsWithTest
  • TakeLastTest
  • TestingSequence
  • TransposeTest
  • ZipLongestTest
  • ZipShortestTest

Copy link
Collaborator

@leandromoh leandromoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@leandromoh
Copy link
Collaborator

leandromoh commented May 3, 2018

I think all the explanation and operator list should be in a issue. So we can create PR for each operator.

@codecov
Copy link

codecov bot commented Nov 30, 2022

Codecov Report

Merging #475 (b39e8d7) into master (1fc1e31) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #475   +/-   ##
=======================================
  Coverage   92.39%   92.39%           
=======================================
  Files         112      112           
  Lines        3447     3447           
  Branches     1021     1021           
=======================================
  Hits         3185     3185           
  Misses        200      200           
  Partials       62       62           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Conflicts resolved:

- MoreLinq.Test/EndsWithTest.cs
- MoreLinq.Test/RankTest.cs
@atifaziz atifaziz changed the title Add UsingTestingSequence to enforce simpler & correct usage Add UsingTestingSequence to enforce simpler & correct usage Jan 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants