Migrate onboarding Cypress tests to Scout #205482
Conversation
mykolaharmash
added
release_note:skip
mykolaharmash
force-pushed
the
migrate-onboarding-cypress-tests
branch
from
740b0d5 to
f11351b
Compare
mykolaharmash
added
the
ci:project-deploy-observability
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
x-pack/solutions/observability/plugins/observability_onboarding/ui_tests/README.md
Outdated
Show resolved
Hide resolved
| test('Users should expand and collapse the Advanced settings section', async ({ | ||
| pageObjects: { customLogsPage }, | ||
| }) => { | ||
| await expect(customLogsPage.namespaceInput()).not.toBeInViewport(); |
There was a problem hiding this comment.
Is using toBeInViewport here intentional or is it a better use case for toBeVisible ?
There was a problem hiding this comment.
yeah, it was a workaround for the way EUI accordion works, but not a very good one. I've changed this test to check the visibility of the whole enclosing container instead of individual input fields, this was I can use toBeVisible()
| page, | ||
| }) => { | ||
| await customLogsPage.integrationNameInput().fill('H3llowOrld'); | ||
| await expect(page.getByText('An integration name should be lowercase.')).toBeVisible(); |
There was a problem hiding this comment.
nit: messages could be extracted out as in
There was a problem hiding this comment.
I've put those messages into a static property of the page object, seems like a mo contextual place for them
| }); | ||
|
|
||
| test.describe('when user is missing privileges', () => { | ||
| test.beforeEach(async ({ browserAuth, page }) => { |
There was a problem hiding this comment.
nit: beforeAll and afterAll here seem more suitable compared to beforeEach and afterEach.
There was a problem hiding this comment.
Agree, but the thing it, Playwright does not allow using page and context (which browserAuth uses) fixtures in beforeAll/afterAll. We'd need to create both fixtures manually, which does not seem to worth it in this case. Playwright doc for reference, just in case.
| logFilePathList() { | ||
| return this.page.locator(`[data-test-subj^=obltOnboardingLogFilePath-]`); | ||
| } |
There was a problem hiding this comment.
IMO, we shouldn't wrap the locators in functions which are not performing any action or are not parametrized. Keeping them as public instance variables would result in cleaner Page Objects, as shown in the example in docs.
@dmlemeshko what are your thoughts on this?
There was a problem hiding this comment.
That's a good point, I'll convert those without any parameters or actions into properties 👍
There was a problem hiding this comment.
Migrated to use properties
mykolaharmash
force-pushed
the
migrate-onboarding-cypress-tests
branch
from
f3f1c8b to
53b9f23
Compare
mykolaharmash
force-pushed
the
migrate-onboarding-cypress-tests
branch
from
4a2ea2c to
c188f73
Compare
|
/oblt-deploy |
|
@awahab07 thank you for a thorough review, I've addressed the points you've mentioned, please take another look |
…playwright config
…arding playwright config
Sorry about this, that was just an error in the script, I'm working on the fix. Did not notice that script initially, I though that modifying the |
💔 Build Failed
Failed CI StepsMetrics [docs]
History
|
This change converts Cypress tests for the custom logs flow into Playwright using the Scout wrapper.
Note
As Scout package is still being developed, the PR pipeline configured to runs Playwright tests only when code in certain plugins have been changed and not on every PR.
How to run tests locally
Start the Scout server
In a separate terminal run the tests
npx playwright test --config x-pack/solutions/observability/plugins/observability_onboarding/ui_tests/playwright.config.tsPlaywright runs browsers in a headless mode by default, user
--headedoption if needed