Kibana: Failing test: X-Pack Jest Tests.x-pack/plugins/index_management/__jest__/client_integration - <TemplateCreate /> on component mount form validation aliases (step 4) should set the correct page title

[kibanamachine]

A test failed on a tracked branch

TypeError: Cannot read property 'data' of undefined
    at getData (/var/lib/jenkins/workspace/elastic+kibana+7.x/kibana/x-pack/plugins/index_management/public/application/components/mappings_editor/mappings_state.tsx:122:52)
    at setDataGetter (/var/lib/jenkins/workspace/elastic+kibana+7.x/kibana/x-pack/plugins/index_management/public/application/components/template_form/steps/step_mappings.tsx:33:22)

First failure: Jenkins Build

[elasticmachine]

Pinging @elastic/kibana-test-triage (failed-test)

[kibanamachine]

New failure: Jenkins Build

[jportner]

Potentially related to / being caused by #55877

[spalger]

Skipped

master: 50471f18ee
7.x/7.7: 655f23ba30

[elasticmachine]

Pinging @elastic/es-ui (Team:Elasticsearch UI)

[thompsongl]

Based on the warnings received when running the template_create suite:

Warning: An update to TemplateForm inside a test was not wrapped in act(...).

Warning: You seem to have overlapping act() calls, this is not supported. Be sure to await previous act() calls before making a new one.

I think the failures are of the same type we saw in #58068, namely that Promise resolution in EuiIcon had been been flushing pending hook effects for the tested component. This can have the effect of artificially keeping the test runner alive, or causing rerenders that would otherwise require act

[sebelga]

@thompsongl is this something that the EUI team should fix? Or should all dependent teams fix their tests? It seems that the former makes more sense but I haven't investigated.

[thompsongl]

@sebelga I'm certainly willing to help. Let me investigate further

[chandlerprall]

is this something that the EUI team should fix? Or should all dependent teams fix their tests?

Starting with some empathy as my next paragraph probably comes off a bit strong: We did miss the introduced flakiness while merging this change and, as Greg said, we're willing to help resolve appropriately.

These errors are outside the scope of EUI. Another way to look at it is if Kibana implemented some 3rd party charting library which artificially kept enzyme/react-test-library alive for a few extra node ticks, and then the charting library optimized to something purely synchronous (but kept the same API surface). We wouldn't say they broke Kibana's tests. As stated earlier, many of these tests were already throwing related warnings but were ignored.

That said, we're all together in this and are happy to help look into related issues where we can, but these issues need to be addressed in Kibana and the individual teams are going to be much, much more effective than Greg or I; e.g. Lens' test issue (#58068) took quite a bit of time from both of us just to understand how the test & underlying components were structured before investigating the cause of the failure.

[cjcenizal]

Thanks @chandlerprall. It sounds like our tests were dependent upon a side effect of EuiIcon's implementation to work. When that side effect disappeared, our tests broke. Do I have that right? If that's the case, then I agree with you. If EUI's interface hasn't changed then we should be responsible for fixing our tests. And thank you for offering to help -- we will be happy to ping you for support!

[thompsongl]

Well put, @chandlerprall. Thanks for jumping in.

It sounds like our tests were dependent upon a side effect of EuiIcon's implementation to work. When that side effect disappeared, our tests broke. Do I have that right?

Correct. The interface for EuiIcon did not change, but an internal async resolution did.

Let me know how I can help!

[sebelga]

Thanks for clarifying @chandlerprall, makes sense now 👍