[Streams] upgrade tests#249617
Conversation
Kerry350
left a comment
There was a problem hiding this comment.
Nice work 👏
Just a question about the location: for the API tests should they still live in the streams_app plugin, or should those move to the streams plugin since that's where the server side code lives?
Good question. I think it probably makes more sense to move them to streams plugin as the tests will be closest to their implementations. |
| // Wait for the data view switcher to be available before selecting | ||
| await page.locator('[data-test-subj*="dataView-switch-link"]').waitFor({ | ||
| state: 'visible', | ||
| timeout: 30_000, | ||
| }); |
There was a problem hiding this comment.
Waiting for this element is already handled inside pageObjects.discover.selectDataView:
const currentValue = await this.page.testSubj.innerText('*dataView-switch-link');
will wait for 10 sec (default) to locate the element.
What I used in other tests, was these 2 methods:
await pageObjects.discover.waitUntilSearchingHasFinished();
await pageObjects.discover.waitForHistogramRendered();
you can use both or at least wait for histogram to render before selecting data view.
| // Wait for the data grid to be fully rendered | ||
| await page.locator('[data-test-subj="discoverDocTable"]').waitFor({ | ||
| state: 'visible', | ||
| timeout: 30_000, | ||
| }); |
There was a problem hiding this comment.
already done with await pageObjects.discover.waitForDocTableRendered()
| // Wait for the data grid to be fully rendered | ||
| await page.locator('[data-test-subj="discoverDocTable"]').waitFor({ | ||
| state: 'visible', | ||
| timeout: 30_000, | ||
| }); |
There was a problem hiding this comment.
await pageObjects.discover.waitForDocTableRendered()
| // Verify the doc viewer flyout is open | ||
| await expect(page.getByTestId('kbnDocViewer')).toBeVisible(); | ||
| // Verify the doc viewer flyout is open (with extended timeout for flyout animation) | ||
| await expect(page.getByTestId('kbnDocViewer')).toBeVisible({ timeout: 30_000 }); |
There was a problem hiding this comment.
what do you think about adding a method in https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-scout/src/playwright/page_objects/discover_app.ts
| await page.getByTestId('streamsAppContentAddFieldButton').click(); | ||
|
|
||
| // Wait `/fields_metadata` so that we are sure the ECS/Otel mapping recommendations are available | ||
| await page.waitForResponse( | ||
| (response) => response.url().includes('/fields_metadata') && response.ok() | ||
| ); | ||
|
|
||
| // Add an Otel field that should have type recommendation (IP type) | ||
| const ecsFieldName = 'resource.attributes.host.ip'; | ||
| await pageObjects.streams.typeFieldName(ecsFieldName); |
There was a problem hiding this comment.
Do we need any extra wait between clicking Add field and start typing to replace removed logic? We saw in the past event handler not being attached fast enough and Kibana acting weird.
| try { | ||
| await esClient.index({ | ||
| index: 'logs', | ||
| document: { | ||
| '@timestamp': new Date().toISOString(), | ||
| message: 'Test document for streams API tests', | ||
| }, | ||
| refresh: true, | ||
| }); | ||
| log.debug('[setup] Test document indexed successfully'); | ||
| } catch (error) { | ||
| log.debug(`[setup] Failed to index test document: ${error}`); | ||
| } |
There was a problem hiding this comment.
question: is it ok to proceed with tests run even if indexing failed? The same question is related to enabling streams
There was a problem hiding this comment.
Makes sense to mark that as a failure.
| try { | ||
| await kbnClient.request({ | ||
| method: 'POST', | ||
| path: '/api/streams/_enable', | ||
| }); | ||
| log.debug('[setup] Streams enabled successfully'); | ||
| } catch (error) { | ||
| log.debug(`[setup] Streams may already be enabled: ${error}`); | ||
| } |
There was a problem hiding this comment.
Afaik we don't need try/catch as it won't throw error if Streams've been already enabled.
We already have this code working:
| api/ | ||
| ├── playwright.config.ts # Scout API test configuration | ||
| ├── constants.ts # Common headers and constants | ||
| ├── fixtures/ | ||
| │ ├── index.ts # Test fixtures extending apiTest | ||
| │ └── constants.ts # User roles and API headers | ||
| ├── services/ | ||
| │ └── streams_api_service.ts # Streams API helper service | ||
| └── tests/ | ||
| ├── global.setup.ts # Global setup (enables Streams) | ||
| ├── processing_simulate.spec.ts | ||
| ├── routing_fork_stream.spec.ts | ||
| ├── schema_field_mapping.spec.ts | ||
| └── lifecycle_retention.spec.ts |
There was a problem hiding this comment.
Happy the day has come :) After UI tests count crossed 200 I was hoping someone will change the pattern and add API tests for Streams. Thank you @CoenWarmer 👏
dmlemeshko
left a comment
There was a problem hiding this comment.
.buildkite/scout_ci_config.yml changes LGTM, I left a few minor comments.
|
Thanks for the review @dmlemeshko, very helpful! |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]
History
|
Summary
This updates Streams App Scout tests.
Details
UI Tests:
API Tests: