-
Notifications
You must be signed in to change notification settings - Fork 3
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
WIP: WEBDEV-6730 fix flaky tests - fixmes #40
Conversation
…ls-and-login Fix and comment flaky e2e details and login
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 think these tests are passing - are we skipping them?
i am not comfortable in merging this as there are many holes still with the tests, many of them are not being run, using fixme
to skip...
q: what happened to smaller PRs? we should talk about approach,
run: npx playwright install --with-deps | ||
|
||
- name: Run about tests | ||
if: always() |
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.
q: do you need the always?
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.
always can be removed, it's just added so the next step will still run even if the previous step has failed
- name: Install Playwright Browsers | ||
run: npx playwright install --with-deps | ||
# - name: Run tests in Webkit browser | ||
# run: npm run test:webkit |
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.
whats the difference between this and the new worflow above?
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.
the new workflow will run the tests by category with specific order... while the other one will run the tests either by browser or all tests without order
@@ -0,0 +1,9 @@ | |||
# This is a basic workflow that is manually triggered |
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.
q: why do we need a manual trigger?
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.
this will help on debugging tests that runs in pipeline vs locally
PATRON_EMAIL: ${{ secrets.PATRON_EMAIL }} | ||
PATRON_PASSWORD: ${{ secrets.PATRON_PASSWORD }} | ||
run: | | ||
${{ github.event.inputs.input_command }} |
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.
q? asking again if we need this manuall lever?
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.
for this one, we want to have a way to run specific tests in pipeline and compare it to the one that was ran locally
}, | ||
privUser: { | ||
email: process.env.A_EMAIL || '', | ||
password: process.env.A_PASSWORD || '', | ||
email: process.env.ARCHIVE_EMAIL || '', |
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.
if this is special, then can we call these vars something else to match their special designation?
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.
yes... but for now it was just what's agreed upon
@@ -68,7 +68,7 @@ test.describe('Profile Page - Basic display tests', () => { | |||
test.describe('Profile Page - Lists', () => { | |||
test.describe.configure({ mode: 'serial' }); | |||
|
|||
test(`Facets appear`, async ({ browser }) => { | |||
test.fixme(`Facets appear`, async ({ browser }) => { |
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.
fixme
here - does this test run?
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.
tests with fixme
annotations will be skipped
await searchPage.collectionFacets.assertSearchFacetGroupCount(); | ||
}); | ||
}); | ||
|
||
test(`Facets for "movies" in Media Type facet group`, async ({ | ||
test.fixme(`Facets for "movies" in Media Type facet group`, 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.
fixme
@@ -31,8 +32,9 @@ test(`Facets for "movies" in Media Type facet group`, async ({ | |||
}); | |||
}); | |||
|
|||
test(`Clear facet filters`, async ({ searchPage }) => { | |||
test.fixme(`Clear facet filters`, async ({ searchPage }) => { |
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.
fixme
); | ||
}); | ||
}); | ||
test.fixme( |
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.
fixme
@@ -120,10 +129,11 @@ test.skip(`Filter for title beginning with "X"`, async ({ searchPage }) => { | |||
}); | |||
}); | |||
|
|||
test(`Facets can be selected via "Select filters" modal`, async ({ | |||
test.fixme(`Facets can be selected via "Select filters" modal`, 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.
fixme
closing this for cleanup |
This branch contains tests with
.fixme()
annotations