-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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.
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
[Uptime] Run Sample uptime tests using @elastic/synthetics #112128
Conversation
611d314
to
12a8125
Compare
Pinging @elastic/uptime (Team:uptime) |
@elasticmachine merge upstream |
@@ -70,5 +68,5 @@ const canvasWebpack = { | |||
module.exports = { | |||
...defaultConfig, | |||
addons: [...(defaultConfig.addons || []), './addon/target/register'], | |||
webpackFinal: (config: Configuration) => webpackMerge(config, canvasWebpack), | |||
webpackFinal: (config) => webpackMerge(config, canvasWebpack), | |||
}; |
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.
build was breaking with ts usage here.
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.
Looking great! Left a few comments for the uptime changes.
// async function refreshUptimeApp() { | ||
// while (!(await page.$('div.euiBasicTable'))) { | ||
// await page.click('[data-test-subj=superDatePickerApplyTimeButton]'); | ||
// // eslint-disable-next-line no-console | ||
// console.log('refreshing uptime app'); | ||
// await page.waitForTimeout(5 * 1000); | ||
// } | ||
// } | ||
// | ||
// async function waitForKibanaToLoad() { | ||
// let isStillLoading = true; | ||
// | ||
// while (isStillLoading) { | ||
// const welcomeMessage = await page.$('text="Welcome to Elastic"'); | ||
// isStillLoading = welcomeMessage === null; | ||
// if (isStillLoading) { | ||
// await page.reload(); | ||
// await page.waitForTimeout(10 * 1000); | ||
// } | ||
// } | ||
// } |
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.
Any reason to keep this?
} | ||
} | ||
|
||
step('Go to Kibana', async () => { |
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.
step('Go to Kibana', async () => { | |
step('Go to Kibana', async () => { |
await page.goto(`${params.kibanaUrl}/app/uptime?dateRangeStart=now-5y&dateRangeEnd=now`, { | ||
waitUntil: 'networkidle', | ||
}); | ||
// await waitForKibanaToLoad(); |
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.
Delete this if there's no reason to keep it.
// await waitForKibanaToLoad(); |
|
||
step('Check if there is table data', async () => { | ||
await page.click('[data-test-subj=uptimeOverviewPage]'); | ||
// await refreshUptimeApp(); |
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.
// await refreshUptimeApp(); |
await page.click('[data-test-subj=monitor-page-link-0001-up]'); | ||
}); | ||
|
||
step('It navigates to details page', async () => { |
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 think we can dispense with naming the step "It..." as that's typically assumed by it
step
test
, etc.
step('It navigates to details page', async () => { | |
step('Navigates to details page', async () => { |
const result = await playwrightStart(getService); | ||
|
||
if (result && result.uptime.status !== 'succeeded') { | ||
process.exit(1); |
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.
Perhaps including an error message here is also a good idea, and perhaps a success message in an else
block?
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.
@elastic/synthetics logs those messages be default, this is just a formality to push the exit code to the CI
} | ||
|
||
async function playwrightStart(getService: any) { | ||
console.log('Loading esArchiver...'); |
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.
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.
defenitely something worth doing in a follow up, probably we will use junit output to hook the logs into jenkins ci
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.
Can we add an enhancement issue then to make sure we don't lose track of it? Should be a fairly light addition someone can implement when they have time.
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.
Sure, i will add that.
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.
LGTM
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 like just changing files from .ts
to .js
to "fix" whatever problem has come up.
The typescript errors look to be originating from the ts-node
package, which is newly introduced by this PR. I would rather we identify why adding the ts-node package suddenly starts breaking this PR build, and to ensure there's not other side effects happening somewhere, rather than just ignore it.
@elasticmachine merge upstream |
webpackFinal: (config: Configuration) => { | ||
return WebpackConfig({ config }); | ||
}, |
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.
webpackFinal usage in storybook main.ts somehow is causing issues with newly added dependency of ts-node
most likely an issue with storybook typescript setup
webpackFinal: (config: Configuration) => { | ||
return WebpackConfig({ config }); | ||
}, |
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.
webpackFinal usage in storybook main.ts somehow is causing issues with newly added dependency of ts-node most likely an issue with storybook typescript setup
@elasticmachine merge upstream |
@elasticmachine merge upstream |
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.
Fleet storybook changes seem good, but would like @clintandrewhall to verify before merging
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.
Fleet changes are fine.
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.
Presentation changes 👍
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / Performance Tests.x-pack/test/performance/tests/reporting_dashboard·ts.performance reporting dashbaord downloaded PDF has OK statusStandard Out
Stack Trace
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @shahzad31 |
…12128) Co-authored-by: Kibana Machine <[email protected]>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
…2128) (#113531) * [Uptime] Run Sample uptime tests using @elastic/synthetics (#112128) Co-authored-by: Kibana Machine <[email protected]> * update test according to 7.x * revert Co-authored-by: Shahzad <[email protected]> Co-authored-by: shahzad31 <[email protected]>
Summary
Fix #112777
This is a follow up to #90673
Integerated @elastic/synthetics based test runner to run uptime e2e tests sample journeys
To RUN These test
from
x-pack/plugins/uptime
Start server using
node scripts/e2e.js --server
after server is up we can start the runner using
node scripts/e2e.js --runner
Runner is based on kibana FTR, but to run tests it's using @elastic/synthetics, which is based on playwright.
Data is a loading using esArchiver utility.
CI
It adds a process which only runs when something changes in
x-pack/plugins/uptime
Next
We would like to migrate few of out uptime tests and user experience app test to this runner to judge capabilities and bottlenecs in the @elastic/synthetics product. It's going to be a great dogfooding.