-
Notifications
You must be signed in to change notification settings - Fork 83
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 for Issue 567 #592
Fix for Issue 567 #592
Conversation
I haven't worked with Emitron in a long time. Is this failure expected? |
I don't know of a reason why the tests would fail. The only issue might be around timeouts for the async tests. Taking a look at the output will give an indication of which tests are failing, which might help narrow down what the problem is. |
Passed here: #593 |
Again, it failed. That's good enough for me for now, but I'd prefer it to be more deterministic… 😑 |
I think the solution is to either come up with a better way to test the async code, or just extend the timeouts. Not sure why they take so long. I guess shared Macs being quite slow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM. With regards to the tests I'm hoping that some of the a/a stuff will help that but yes the tests are causing lots off issues with CI and PRs. We could investigate wrapping the tests that need to wait with helpers to automatically retry or remove the source of asynchronicity and replacing it with something we can stub out in tests to control better
#567