-
Notifications
You must be signed in to change notification settings - Fork 1.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
Mct7367-tests #7387
Mct7367-tests #7387
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #7387 +/- ##
==========================================
- Coverage 55.89% 55.66% -0.23%
==========================================
Files 659 659
Lines 26277 26277
Branches 2549 2549
==========================================
- Hits 14687 14628 -59
- Misses 10882 10940 +58
- Partials 708 709 +1
... and 21 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
All that's left is role=progressbar |
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.
Awesome ! I just had one review comment from a pending review, and this needs a good linting. Should be g2g after that
@@ -25,6 +25,10 @@ | |||
class="c-progress-bar__bar" | |||
:class="{ '--indeterminate': !progressPerc }" | |||
:style="styleBarWidth" | |||
role="progressbar" | |||
:aria-valuenow="progressPerc" |
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.
woah neat
@@ -36,7 +36,7 @@ test.describe('Testing numeric data with inspector data visualization (i.e., dat | |||
await page.goto('./', { waitUntil: 'domcontentloaded' }); | |||
}); | |||
|
|||
test('Can click on telemetry and see data in inspector', async ({ page, context }) => { | |||
test('Can click on telemetry and see data in inspector @2p', async ({ page, context }) => { |
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.
driveby for @scottbell
|
||
const NOTEBOOK_NAME = 'Notebook'; | ||
|
||
test.describe('Snapshot image tests', () => { |
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.
broke this out into its own suite for speed
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.
doesn't it parallelize by file and not suite? or has that changed
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.
it's by file. I broke up the image and regular snapshot suites
@@ -91,22 +85,13 @@ test.describe('Snapshot Container tests', () => { | |||
|
|||
await page.getByLabel('Take a Notebook Snapshot').click(); | |||
await page.getByRole('menuitem', { name: 'Save to Notebook Snapshots' }).click(); | |||
await page.getByRole('button', { name: 'Show' }).click(); | |||
await page.getByLabel('Show Snapshots').click(); |
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.
locator fixes
@@ -50,6 +50,17 @@ test.describe('Visual - Header @a11y', () => { | |||
scope: header | |||
}); | |||
}); | |||
|
|||
test('show snapshot button', async ({ page, theme }) => { |
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.
realized we were missing this one
await percySnapshot(page, `Notebook Snapshot Show button (theme: '${theme}')`, { | ||
scope: header | ||
}); | ||
await expect(await page.getByLabel('Show Snapshots')).toBeVisible(); |
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.
need to wait for button to appear in header
let folder; | ||
test.beforeEach(async ({ page }) => { | ||
// Go to baseURL | ||
await page.goto('./', { waitUntil: 'networkidle' }); |
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.
networkidle?
Describe your changes:
Tests for the #7367 feature
All Submissions:
Author Checklist
type:
label? Note: this is not necessarily the same as the original issue.Reviewer Checklist