Skip to content

[CCR] Unskip follower indices component integration test#120912

Merged
sebelga merged 10 commits intoelastic:mainfrom
sebelga:tests/unskip-cit-ccr-follower-indices
Dec 14, 2021
Merged

[CCR] Unskip follower indices component integration test#120912
sebelga merged 10 commits intoelastic:mainfrom
sebelga:tests/unskip-cit-ccr-follower-indices

Conversation

@sebelga
Copy link
Copy Markdown
Contributor

@sebelga sebelga commented Dec 9, 2021

I have unskiped the test which works without issue locally. I removed a few unnecessary async function and tried to run the tests in a loop (20 times) but that gave me the following strange (and hard to debug) jest errors

var evt = document.createEvent('Event'); // Keeps track of whether the user-provided callback threw an error. We
                         ^
TypeError: Cannot read properties of null (reading 'createEvent')
    at Object.invokeGuardedCallbackDev (/Users/sebastienloix/Workspace/elastic/kibana-instance-4/node_modules/react-dom/cjs/react-dom.development.js:302:26)
    at invokeGuardedCallback (/Users/sebastienloix/Workspace/elastic/kibana-instance-4/node_modules/react-dom/cjs/react-dom.development.js:440:31)

The above error only appears if I loop more than twice the tests on my machine, maybe something memory related?

We'll run multiple times CI on this PR to make sure that flakiness is gone.

Fixes #100951

@sebelga sebelga requested a review from a team as a code owner December 9, 2021 14:30
@sebelga sebelga added release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// test v8.0.0 v8.1.0 labels Dec 9, 2021
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 9, 2021

Pinging @elastic/kibana-stack-management (Team:Stack Management)

@sebelga sebelga requested a review from yuliacech December 9, 2021 14:32
@sebelga
Copy link
Copy Markdown
Contributor Author

sebelga commented Dec 9, 2021

@elasticmachine merge upstream

yuliacech
yuliacech reviewed Dec 9, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

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

Thank you for un-skipping this test, @sebelga!
Code changes LGTM 👍 I think I saw a special command to run PR tests multiple times for flakiness. @cuff-links Do you maybe know how to do that automatically?

@sebelga
Copy link
Copy Markdown
Contributor Author

sebelga commented Dec 9, 2021

@elasticmachine merge upstream

@sebelga
Copy link
Copy Markdown
Contributor Author

sebelga commented Dec 9, 2021

I think I saw a special command to run PR tests multiple times for flakiness

I asked the operation team and according to them the only way is to execute the tests in a loop which did not work in this case. Spent quite a bit of time on it but couldn't make the error above go away. it only appears when executing jest tests in a loop which makes me think that it is jest related and not test related.

@sebelga
Copy link
Copy Markdown
Contributor Author

sebelga commented Dec 13, 2021

@elasticmachine merge upstream

@sebelga
Copy link
Copy Markdown
Contributor Author

sebelga commented Dec 13, 2021

@elasticmachine merge upstream

yuliacech
yuliacech approved these changes Dec 13, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

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

Thanks a lot, @sebelga! Should we ope an issue for the jest problem you discovered?
I'm happy with un-skipping this test, but if the jest problems persists it might fail at some point again.

@sebelga
Copy link
Copy Markdown
Contributor Author

sebelga commented Dec 13, 2021

Thanks @yuliacech. Jest tests should be deterministic as they don't depend on other system. We used to have problems in the past when we didn't use the jest.useFaketimers and our tests were timing out because of internal timer issue with Jest parallelisation when running tests. I checked the current tests and they don't use any real timers.

So if running 10 times CI on this PR succeeds (and many other runs locally) I would assume that any flakyness comes from the infrastructure and not the tests themselves. The op team acknowledged that they had issues with jest a few months ago and the fixed it.

Should we ope an issue for the jest problem you discovered?

Do you mean on the jest repository? I am not sure yet that the effort is worth it. I could only reproduce that error when putting the whole test suits under a loop of 3 or more iterations. Removing the loop didn't cause any issue.

@sebelga
Copy link
Copy Markdown
Contributor Author

sebelga commented Dec 13, 2021

@elasticmachine merge upstream

@sebelga
Copy link
Copy Markdown
Contributor Author

sebelga commented Dec 13, 2021

@elasticmachine merge upstream

@cuff-links
Copy link
Copy Markdown
Contributor

cuff-links commented Dec 13, 2021

Thank you for un-skipping this test, @sebelga! Code changes LGTM 👍 I think I saw a special command to run PR tests multiple times for flakiness. @cuff-links Do you maybe know how to do that automatically?

Hi Yulia. The only tests that I know that we can run repeatedly like that are functional test runner tests. I will double check that and report back.

@cuff-links
Copy link
Copy Markdown
Contributor

cuff-links commented Dec 13, 2021

@yuliacech Confirmed. Only works with functional test runner.

@sebelga
Copy link
Copy Markdown
Contributor Author

sebelga commented Dec 13, 2021

@elasticmachine merge upstream

This was referenced Dec 13, 2021
Merged
Merged
Merged
@sebelga
Copy link
Copy Markdown
Contributor Author

sebelga commented Dec 14, 2021

@elasticmachine merge upstream

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Dec 14, 2021

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@sebelga sebelga merged commit 36aece0 into elastic:main Dec 14, 2021
@sebelga sebelga deleted the tests/unskip-cit-ccr-follower-indices branch December 14, 2021 16:12
sebelga added a commit to sebelga/kibana that referenced this pull request Dec 14, 2021
@sebelga
[CCR] Unskip follower indices component integration test (elastic#120912
897c2c5 
)
@sebelga sebelga mentioned this pull request Dec 14, 2021
Merged
sebelga added a commit to sebelga/kibana that referenced this pull request Dec 14, 2021
@sebelga
[CCR] Unskip follower indices component integration test (elastic#120912
3033920 
)
@sebelga sebelga mentioned this pull request Dec 14, 2021
Merged
sebelga added a commit that referenced this pull request Dec 14, 2021
@sebelga
[CCR] Unskip follower indices component integration test (#120912) (#…
90e290d 
…121206)
sebelga added a commit that referenced this pull request Dec 14, 2021
@sebelga
[CCR] Unskip follower indices component integration test (#120912) (#…
6e510f8 
…121207)
TinLe pushed a commit to TinLe/kibana that referenced this pull request Dec 22, 2021
@sebelga @TinLe
[CCR] Unskip follower indices component integration test (elastic#120912
abcb35b 
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@yuliacech yuliacech yuliacech approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// test v7.16.2 v8.0.0 v8.1.0

Projects

None yet

Milestone

No milestone

6 participants

@sebelga @elasticmachine @cuff-links @kibana-ci @yuliacech @kibanamachine