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

Fix debounce when started twice #772

Merged
merged 4 commits into from
Feb 6, 2020

Conversation

gpambrozio
Copy link
Contributor

@gpambrozio gpambrozio commented Feb 2, 2020

Fixes issue #771. Thanks @andersio for the quick solution on this.

I also added tests that show the solution works. The tests were added at a commit before the change that fixed it so it's easy to verify that tests fail before the change but not after.

Checklist

  • Updated CHANGELOG.md.

@gpambrozio
Copy link
Contributor Author

@andersio not sure why this test failed ci:

/Users/travis/build/ReactiveCocoa/ReactiveSwift/Tests/ReactiveSwiftTests/SignalProducerSpec.swift:1060: error: -[ReactiveSwiftTests.SignalProducerSpec zip, can deal with hundreds of producers] : failed - Waited more than 1.0 second

It seems go only fail sometimes when I run the tests locally, might be indicative of a performance issue or just poor cpu management at the time it ran.

Adding a (timeout: 2) to that test makes it succeed all the time for me, even when my machine is building another project at the same time but I don't want to push this "fix" without getting some thoughts on it first.

@andersio
Copy link
Member

andersio commented Feb 4, 2020

@gpambrozio It is general flakiness of asynchronous test IMO. It can be rewritten it in terms of TestScheduler at a later point.

@gpambrozio
Copy link
Contributor Author

@andersio do you want me to add the timeout change to this PR?

@andersio
Copy link
Member

andersio commented Feb 6, 2020

@gpambrozio It wouldn't be necessary. 👍

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.

None yet

2 participants