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.

Sign up for GitHub

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

Jump to bottom

[Endpoint] FIX: Increase saga tests sleep() default duration back to 100ms #55492

Merged
merged 1 commit into from
Jan 22, 2020
Merged

[Endpoint] FIX: Increase saga tests sleep() default duration back to 100ms #55492

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions x-pack/plugins/endpoint/public/applications/endpoint/lib/saga.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,12 @@
import { createSagaMiddleware, SagaContext, SagaMiddleware } from './index';
import { applyMiddleware, createStore, Reducer, Store } from 'redux';

// Failing: https:/elastic/kibana/issues/55464 https:/elastic/kibana/issues/55465
describe.skip('saga', () => {
describe('saga', () => {
const INCREMENT_COUNTER = 'INCREMENT';
const DELAYED_INCREMENT_COUNTER = 'DELAYED INCREMENT COUNTER';
const STOP_SAGA_PROCESSING = 'BREAK ASYNC ITERATOR';

const sleep = (ms = 10) => new Promise(resolve => setTimeout(resolve, ms));
const sleep = (ms = 100) => new Promise(resolve => setTimeout(resolve, ms));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm worried about this continuing to be a problem down the road. I assume 10 was intermittent depending on resources during CI. 100 would certainly make it occur less often, but kibana is a constantly growing application. Just weighing the values of this test vs. the risk of intermittent failures. What are your thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I looked at this for a while yesterday trying to see if there was another way. Because this lib is the core for us doing side-effects, I think there is high value in ensuring that it behaves as expected. I searched around other x-pack plugins and I do see 100 (or in some cases higher values) being used. Because there are not allot of tests here, I can also increase the ms to a higher number in order to try to avoid this type of failure again.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, thanks for the reply and taking a look around at the other plugins.

let store: Store;
let reducerA: Reducer;
let sideAffect: (a: unknown, s: unknown) => void;
Expand Down