Ensure TCK passes with empty publisher that synchronously completes

[jroper]

Fixes #422

[akarnokd]

There is a completeImmediately() method on the promise.

[akarnokd]

Btw. the plain complete() behavior is odd. I'd think once called, isCompleted() should report true but the implementation queues the object.

[jroper]

I agree - but I'm not familiar enough with the TCK to change it. Though I can understand why the event is queued, so that the order of events can be verified and duplicates can be detected etc.

[ktoso]

I'll investigate this

[ktoso]

Thanks for noticing, I think we can improve the impl there. I'll work on it today after done with work stuff.
I'll PR into this PR then :)

[ktoso]

I reimplemented the Promise which makes this workwithout the additional atomic.
Please see here: jroper#1

[viktorklang]

Thanks @jroper!

@Override
     public void onNext(T element) {
       env.debug(String.format("%s::onNext(%s)", this, element));
       if (subscription.isCompleted()) { // Shouldn't this be onSubscribeCalled.get() too? (for subscribe + onSubscribe + sync request(n) + onNext?
         super.onNext(element);
       } else {
         env.flop(String.format("Subscriber::onNext(%s) called before Subscriber::onSubscribe", element));
       }
     }

[jroper]

I thought about changing onNext, however, the manual subscriber will never invoke Subscription.request before invoking expectCompletion, so I think that logic is still valid, even if the error message is a little odd. My worry with changing it is that there might be some tests that depend on the current behaviour.

[viktorklang]

Repackage Errors as Exceptions?

[jroper]

Just being consistent with requireTestFailure and requireTestSkip.

[ktoso]

Does not really matter much as those will end up failing the test in the suite and one may always look into the logs. It's not like it's a production app where we have to watch out to not accidentally continue after an OOM or something... Fine as is I think

[ktoso]

I submitted an impl improvement of the Promise which makes the test pass the new requirement:
jroper#1

I submitted the change as PR into this PR, so if you accept it over there @jroper we could merge all those commits rebased here together I guess.

[viktorklang]

@jroper @ktoso Hey all, I just wanted to know what the current status is here.

[ktoso]

I would like @jroper to merge the fix I proposed to this PR, it's been waiting ;-)
jroper#1

[ktoso]

PR was merged, thanks @jroper ! Seems good to go from my side then

[viktorklang]

thanks @jroper!