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 Future Order tests #895

Closed
wants to merge 1 commit into from
Closed

Conversation

adelbertc
Copy link
Contributor

No description provided.

@codecov-io
Copy link

Current coverage is 89.05%

Merging #895 into master will not affect coverage as of b60a213

@@            master    #895   diff @@
======================================
  Files          174     174       
  Stmts         2384    2384       
  Branches        76      76       
  Methods          0       0       
======================================
  Hit           2123    2123       
  Partial          0       0       
  Missed         261     261       

Review entire Coverage Diff as of b60a213

Powered by Codecov. Updated on successful CI builds.

@non
Copy link
Contributor

non commented Feb 25, 2016

👍

@ceedubs
Copy link
Contributor

ceedubs commented Mar 2, 2016

Thanks @adelbertc. I'm not so sure about this because of #589. Have you taken a look at that issue?

@adelbertc
Copy link
Contributor Author

@ceedubs Just read through it again - the problem is the unlawfulness assumes side effects right? e.g. throwing exceptions, at which point we can't say anything about anything. In the absence of such things it appears to work

@ceedubs
Copy link
Contributor

ceedubs commented Mar 3, 2016

@adelbertc I don't think I quite see it that way. A failed Future is a perfectly valid case of a Future (similar to an Xor.Left), and it can be created without throwing exceptions. It's the Order[Future] instance that throws for a perfectly valid Future and therefore is the problematic piece (beyond blocking) in my opinion.

@adelbertc
Copy link
Contributor Author

Ah fair.. in which case these test are bogus I think.

@adelbertc adelbertc closed this Mar 3, 2016
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.

4 participants