Make force work in conditions for futures, not only promises. #27
Conversation
|
This seems valid at first glance. Can you create a test that demonstrates the problem? |
|
The test case is the example above - it prints all three lines in case of futures, while only the first line for promises. Should be trivial to convert to the appropriate assert/expect/should/whatever. I would need it for construct like this which I sometimes use for running things in parallel: It works almost fine now, except that nil/false results are not properly handled in the resulting array, depending on how they are use. |
I'd need to see these added to the specs in the code. Note that travis is failing.
Can I ask why you're doing this? The point of future is that they start running immediately. The second |
The Future#__force__ should return the resulting value, not the promise. Otherwise things like
``` ruby
puts "true" if future{ true }.force
puts "false" if future{ false }.force
puts "nil" if future{ nil }.force
```
do not work as expected (currently all evaluate to true). I believe it should work consistently with promises:
``` ruby
puts "true" if promise{ true }.force
puts "false" if promise{ false }.force
puts "nil" if promise{ nil }.force
```
The change above should achieve this. However note it may change how the marshalling works with regard to futures, as the special marshalling handling found in Promise becomes bypassed. You may want to review this aspect yourself.
|
I have added the tests to the specs. As for the travis failing, that's because the shared spec tests for compatibility with Marshal. However, Marshalling of futures doesn't work anyway (and didn't before), as Future class lacks the _load method and Marshal.load fails because of that. So I adjusted the spec accordingly. The alternative would be adding proper Marshalling support to Future class itself, basically cloning the functionality from Promise class. As for the |
|
Hmm, the travis tests now pass for everything but JRuby, but checking the log it seems travis has problems running bundler under JRuby, rather than the tests themselves failing. I can't see how this can be fixed other than trying running the tests on travis again later. |
The Future#force should return the resulting value, not the promise. Otherwise things like
do not work as expected (currently all evaluate to true). I believe it should work consistently with promises:
The change above should achieve this. However note it may change how the marshalling works with regard to futures, as the special marshalling handling found in Promise becomes bypassed. You may want to review this aspect yourself.