[Discover] Unskip Discover-Lens functional test suite #200687
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -203,6 +203,22 @@ export function Histogram({ | |
| transform: translate(-50%, -50%); | ||
| } | ||
| `; | ||
| // console.log('histogram', { lensProps, requestData, request, dataView, visContext }); | ||
| if ( | ||
| dataView.id && | ||
| (dataView.id !== visContext.requestData.dataViewId || | ||
| (!dataView.isPersisted() && !lensProps.attributes.state.adHocDataViews?.[dataView.id])) | ||
| ) { | ||
| // to prevent a race condition when the data view changes id | ||
| // the ids of the data view and the lens props and visContextDataView should be the same | ||
| // console.log('flakydebugging', { | ||
| // dataViewId: dataView.id, | ||
| // visContextDataViewId: visContext.requestData.dataViewId, | ||
| // lensProps: lensProps.attributes.references.find((r) => r.id === dataView.id), | ||
| // }); | ||
|
|
||
| return <></>; | ||
| } | ||
|
There was a problem hiding this comment. So, this seems to prevent the history to fail, because we have different data views in different props that are passed into the Lens embeddable. Ideally this should be caught on a higher level, so this kind of situation can't happen. So just display a loading state until props are aligned There was a problem hiding this comment. Let's unskip the test suite and run the flaky test runner too. There was a problem hiding this comment. Wow! Thx! This is great news! There was a problem hiding this comment. Also, another update to tests would be necessary once you unskip them as in e9d5c25 There was a problem hiding this comment. tests are green, and flaky test runner too |
||
|
|
||
| return ( | ||
| <> | ||
|
|
||
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.
@jughosta do we have an alternative here? we need to prevent the rendering for the flaky case, should we render something, given we move forward with this PR (it woulld need to be investigated or refactored later on, since the given state causing the flakiness ideally should not happen)
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.
I don't have an alternative right now. What I've tried was causing other test failures and extra fetches. So let's go with your solution.
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.
Ok, so we should open an issue to remove this code, given we resolve the root cause, which might be a bigger refactoring