search

ReactiveX / RxSwing

RxJava bindings for Swing
Apache License 2.0
98 stars 23 forks source link

Fixed broken KeyEventSourceTest. #54

Closed mikebaum closed 8 years ago

mikebaum commented 8 years ago

Fixes issue #53

mikebaum commented 8 years ago

@akarnokd I wonder if you could review this small change. It seems that you're recent fix for: github/rxjava#3168 broke a unit test in RxSwing. I just wanted to make sure that it is indeed intended that scan when called with an initial value always emits that initial value regardless of whether or not the source observable is a BehaviorSubject.

akarnokd commented 8 years ago

Yes, the initial value was supposed to be fired immediately and not in conjunction with the first regular event.

mikebaum commented 8 years ago

@jmhofer Since, the scan operator has been updated to always emit the initial value the test testObservingPressedKeys is now failing. The test is failing since the method KeyEventSource.currentlyPressedKeysOf() uses a scan operator internally.

This PR fixes the test, but perhaps the better fix would be to make sure that the method KeyEventSource.currentlyPressedKeysOf() returns an Observable that does not emit the first value. If you feel it is okay that this observable emits a first item that is an empty list, then there is nothing more to do. Otherwise we could fix the method to return an Observable that does not emit an initial empty list. Let me know your thoughts.

mikebaum commented 8 years ago

I'm going to merge and close this PR, since this failing test is currently blocking any other PRs. @jmhofer If you feel strongly enough that the new behaviour of the KeyEventSource.currentlyPressedKeysOf() is unacceptable, then we can issue another PR to update that method.