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

Fix StreamingT.filter bug #798

Merged
merged 1 commit into from
Jan 12, 2016
Merged

Conversation

ceedubs
Copy link
Contributor

@ceedubs ceedubs commented Jan 12, 2016

As reported by @liff here.

@codecov-io
Copy link

Current coverage is 88.30%

Merging #798 into master will not affect coverage as of 5cb519f

@@            master   #798   diff @@
=====================================
  Files          166    166       
  Stmts         2291   2292     +1
  Branches        75     75       
  Methods          0      0       
=====================================
+ Hit           2023   2024     +1
  Partial          0      0       
  Missed         268    268       

Review entire Coverage Diff as of 5cb519f

Powered by Codecov. Updated on successful CI builds.

@erik-stripe
Copy link
Contributor

Wow! Jeez. Thanks @liff for catching this (and @ceedubs for the fix).

👍

@fthomas
Copy link
Member

fthomas commented Jan 12, 2016

Any idea why this wasn't caught by the "filter with Id consistent with List.filter" test?

@ceedubs
Copy link
Contributor Author

ceedubs commented Jan 12, 2016

@fthomas I think because Scalacheck's auto-generated functions completely ignore their input and return a constant, so either the whole stream was filtered (false) or none of it was filtered (true). The effectiveness of these sorts of tests should improve once there is a new release of Scalacheck.

@xuwei-k
Copy link
Contributor

xuwei-k commented Jan 12, 2016

LGTM xuwei-k@06e542d8f60583024fe902

@ceedubs
Copy link
Contributor Author

ceedubs commented Jan 12, 2016

Thanks @xuwei-k!

I'm going to go ahead and merge since this is a pretty obvious/straightforward bug.

ceedubs added a commit that referenced this pull request Jan 12, 2016
@ceedubs ceedubs merged commit ceff342 into typelevel:master Jan 12, 2016
@ceedubs ceedubs deleted the streamingt-filter-bug branch January 12, 2016 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants