2.x collect - handle post terminal events

[davidmoten]

Just looking at moving some recent 1.x bug fixes and tests over to 2.x.

This is my first PR in 2.x so before I proceed adding more tests from 1.x (to this PR) I want to get some feedback on my approach. In particular in the test testCollectorFailureDoesNotResultInTwoErrorEmissions is the Flowable creation method ok?

[akarnokd]

I'd instead throw TestException("Forced failure") and then simply

Flowable.just(1).collect(...)
.test()
.assertFailureAndMessage(TestException.class, "Forced failure");

[davidmoten]

Great, I love the .test method on this!

[akarnokd]

If you'd make it more concise, that would be great. Also note that most Flowable operators have an Observable counterpart so it would be nice if such changes would contain the fix & tests for both.

[codecov-io]

Current coverage is 69.33% (diff: 100%)

Merging #4364 into 2.x will increase coverage by 0.05%

@@                2.x      #4364   diff @@
==========================================
  Files           454        454          
  Lines         32483      32492     +9   
  Methods           0          0          
  Messages          0          0          
  Branches       5204       5207     +3   
==========================================
+ Hits          22504      22529    +25   
+ Misses         7776       7764    -12   
+ Partials       2203       2199     -4   

Powered by Codecov. Last update 7a1a4af...2931f24

[davidmoten]

If you'd make it more concise, that would be great. Also note that most Flowable operators have an Observable counterpart so it would be nice if such changes would contain the fix & tests for both.

Can do.

[akarnokd]

I was trying to avoid this class till now. Will you be able to finish what you wanted till Thursday or may I do it?

[davidmoten]

@akarnokd I rebased but io.reactivex.Optional disappeared and I was using it in tests. I'll add it back in internal test package.

[davidmoten]

@akarnokd the work is done for Flowable and I have done the work for Observable as well but is on my laptop at home so will finish PR tonight (in about 11 hours time).

[davidmoten]

@akarnokd I'm noticing that RxJava 2.x is really slow to compile (2 minutes) in Eclipse 4.5.1 and 4.6 (say after switching branches). Have you noticed that? Got any tips?

[akarnokd]

Please try without Optional. Yes Eclipse is slow if you change Observable and Flowable, it doesn't like the cycle between the base classes and their implementations. Not sure why, maybe collecting metadata makes is much slower.

[davidmoten]

I've removed Optional. Would you like to assess this PR as it stands without the Observable fix that I can do in another PR? You guys are moving fast (as usual) and keeping PRs open a while is a bit of an obstacle.

[akarnokd]

👍