-
Notifications
You must be signed in to change notification settings - Fork 2
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
test: add snapshots to graph related tests also disable chromatic on geneset names and tooltips #802
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #802 +/- ##
=======================================
Coverage 77.47% 77.47%
=======================================
Files 88 88
Lines 6856 6856
=======================================
Hits 5312 5312
Misses 1544 1544
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
const imageID = "graph-image"; | ||
await page.getByTestId(buttonID).click(); | ||
page.getByTestId(imageID).waitFor(); | ||
await takeSnapshot(page, testInfo); |
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'm making the assumption that chromatic will offer a diff from this?
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.
that's correct! Chromatic saves the latest approved snapshot in its own DB to diff with each PR!
client/__tests__/e2e/e2e.test.ts
Outdated
await setup(option, page); | ||
await createGeneset(meanExpressionBrushGenesetName, page); | ||
await addGeneToSetAndExpand(meanExpressionBrushGenesetName, "SIK1", page); | ||
|
||
await colorByGeneset(meanExpressionBrushGenesetName, page); | ||
await assertColorLegendLabel(meanExpressionBrushGenesetName, page); | ||
snapshotTestGraph(page, testInfo); |
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.
adding snapshot
client/__tests__/e2e/e2e.test.ts
Outdated
@@ -379,6 +385,7 @@ describe("ui elements don't error", () => { | |||
await drag("layout-graph", panCoords.start, panCoords.end, page); | |||
|
|||
await page.evaluate("window.scrollBy(0, 1000);"); | |||
snapshotTestGraph(page, testInfo); |
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.
adding snapshot
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.
looks prettier ran its course on the file, ive commented the actual diffs
@@ -1,5 +1,4 @@ | |||
import { ReporterDescription, defineConfig, devices } from "@playwright/test"; | |||
import { COMMON_PLAYWRIGHT_CONTEXT } from "./__tests__/common/playwrightContext"; |
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.
deleted this constant in favor of declaring it in this file
data-testid={`geneset-description-tooltip-${originalString}${tooltipAddendum}`} | ||
data-chromatic="ignore" |
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.
chromatic ignore
@@ -128,6 +128,7 @@ class GeneSet extends React.Component<{}, State> { | |||
maxWidth: globals.leftSidebarWidth - genesetNameLengthVisible, | |||
}} | |||
data-testid={`${setName}:geneset-name`} | |||
data-chromatic="ignore" |
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.
chromatic ignore
be277cc
to
8fa8dc4
Compare
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.
Code looks good! Thanks for setting this up, @seve 🎉
Probably just need to pass smoke test, so we can see the new diff in Chromatic 🤞
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.
Sorry didn't LGTM it last time lol
0efea50
to
1325cfc
Compare
#716 #760
Adds a util to take a snapshot of the graph for doing visual diff testing. Also prevents chromatic from recognizing the geneset names as diffs