Skip to content

Commit

Permalink
Fix visual testing flakiness (#2750)
Browse files Browse the repository at this point in the history
  • Loading branch information
@gregberge
gregberge authored Jan 14, 2025
1 parent 1de9d1a commit e000f23
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 27 deletions.
107 changes: 87 additions & 20 deletions packages/gitbook/e2e/pages.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,40 @@ import { getContentTestURL } from '../tests/utils';

interface Test {
name: string;
url: string; // URL to visit for testing
/**
* URL to visit for testing.
*/
url: string;
cookies?: Parameters<BrowserContext['addCookies']>[0];
run?: (page: Page) => Promise<unknown>; // The test to run
fullPage?: boolean; // Whether the test should be fullscreened during testing
screenshot?: false | { threshold: number }; // Disable screenshot or set threshold
only?: boolean; // Only run this test
/**
* Test to run
*/
run?: (page: Page) => Promise<unknown>;
/**
* Whether the test should be fullscreened during testing.
*/
fullPage?: boolean;
/**
* Whether to take a screenshot of the test or set a threshold for the screenshot.
*/
screenshot?:
| false
| {
/**
* Screenshot threshold.
* From 0 to 1, where 0 is the most strict and 1 is the most permissive.
* @default 0.5
*/
threshold?: number;
/**
* Whether to wait for the table of contents to finish scrolling before taking the screenshot.
*/
waitForTOCScrolling?: boolean;
};
/**
* Whether to only run this test.
*/
only?: boolean;
}

interface TestsCase {
Expand Down Expand Up @@ -323,6 +351,9 @@ const testCases: TestsCase[] = [
{
name: 'PDF',
url: '~gitbook/pdf?limit=10',
screenshot: {
waitForTOCScrolling: false,
},
},
],
},
Expand All @@ -345,6 +376,7 @@ const testCases: TestsCase[] = [
url: 'blocks/block-images',
run: waitForCookiesDialog,
fullPage: true,
screenshot: { threshold: 0.8 },
},
{
name: 'Inline Images',
Expand Down Expand Up @@ -421,6 +453,18 @@ const testCases: TestsCase[] = [
{
name: 'Math',
url: 'blocks/math',
run: async (page) => {
await page.waitForFunction(() => {
const fonts = Array.from(document.fonts.values());
const mjxFonts = fonts.filter(
(font) => font.family === 'MJXZERO' || font.family === 'MJXTEX',
);
return (
mjxFonts.length === 2 &&
mjxFonts.every((font) => font.status === 'loaded')
);
});
},
},
{
name: 'Files',
Expand Down Expand Up @@ -474,6 +518,9 @@ const testCases: TestsCase[] = [
name: 'With cover and no TOC',
url: 'page-options/page-with-cover-and-no-toc',
run: waitForCookiesDialog,
screenshot: {
waitForTOCScrolling: false,
},
},
{
name: 'With icon',
Expand Down Expand Up @@ -657,11 +704,14 @@ const testCases: TestsCase[] = [
run: async (page) => {
const sharedSpaceLink = page.locator('a.underline');
await sharedSpaceLink.click();
expect(page.locator('h1')).toHaveText('shared');
await expect(
page.getByRole('heading', { level: 1, name: 'shared' }),
).toBeVisible();
const url = page.url();
expect(url.includes('shared-space-uno')).toBeTruthy(); // same uno site
expect(url.endsWith('/shared')).toBeTruthy(); // correct page
},
screenshot: false,
},
],
},
Expand All @@ -673,13 +723,15 @@ const testCases: TestsCase[] = [
name: 'Navigation to shared space',
url: '',
run: async (page) => {
const sharedSpaceLink = page.locator('a.underline');
await sharedSpaceLink.click();
expect(page.locator('h1')).toHaveText('shared');
await page.locator('a.underline').click();
await expect(
page.getByRole('heading', { level: 1, name: 'shared' }),
).toBeVisible();
const url = page.url();
expect(url.includes('shared-space-dos')).toBeTruthy(); // same dos site
expect(url.endsWith('/shared')).toBeTruthy(); // correct page
},
screenshot: false,
},
],
},
Expand Down Expand Up @@ -713,6 +765,7 @@ const testCases: TestsCase[] = [
page.getByText('Authentication missing to access this content'),
).toBeVisible();
},
screenshot: false,
},
],
},
Expand Down Expand Up @@ -1377,20 +1430,23 @@ for (const testCase of testCases) {
if (testEntry.run) {
await testEntry.run(page);
}
if (testEntry.screenshot !== false) {
await scrollTOCToTop(page);
const screenshotOptions = testEntry.screenshot;
if (screenshotOptions !== false) {
await argosScreenshot(page, `${testCase.name} - ${testEntry.name}`, {
viewports: ['macbook-16', 'macbook-13', 'iphone-x', 'ipad-2'],
viewports: ['macbook-16', 'macbook-13', 'ipad-2', 'iphone-x'],
argosCSS: `
/* Hide Intercom */
.intercom-lightweight-app {
display: none !important;
}
`,
threshold: testEntry.screenshot?.threshold ?? undefined,
}
`,
threshold: screenshotOptions?.threshold ?? undefined,
fullPage: testEntry.fullPage ?? false,
beforeScreenshot: async () => {
await waitForIcons(page);
if (screenshotOptions?.waitForTOCScrolling !== false) {
await waitForTOCScrolling(page);
}
},
});
}
Expand Down Expand Up @@ -1501,10 +1557,21 @@ async function waitForIcons(page: Page) {
}

/**
* Scroll the table of contents to the top to stabilize the screenshot.
* Wait for TOC to be correctly scrolled into view.
*/
async function scrollTOCToTop(page: Page) {
await page.evaluate(() => {
document.querySelector('[data-testid=toc-container]')?.scrollTo(0, 0);
});
async function waitForTOCScrolling(page: Page) {
const viewport = await page.viewportSize();
if (viewport && viewport.width >= 1024) {
const toc = page.getByTestId('table-of-contents');
await expect(toc).toBeVisible();
await page.evaluate(() => {
const tocScrollContainer = document.querySelector(
'[data-testid="table-of-contents"] [data-testid="toc-scroll-container"]',
);
if (!tocScrollContainer) {
throw new Error('TOC scroll container not found');
}
tocScrollContainer.scrollTo(0, 0);
});
}
}
1 change: 1 addition & 0 deletions packages/gitbook/src/components/DocumentView/Embed.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export async function Embed(props: BlockProps<gitbookAPI.DocumentBlockEmbed>) {
dangerouslySetInnerHTML={{
__html: embed.html,
}}
data-visual-test="blackout"
/>
<Script src="https://cdn.iframe.ly/embed.js" nonce={nonce} />
</>
Expand Down
2 changes: 1 addition & 1 deletion packages/gitbook/src/components/TableOfContents/TOCScroller.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export function TOCScrollContainer(props: {
<TOCScrollContainerRefContext.Provider value={scrollContainerRef}>
<div
ref={scrollContainerRef}
data-testid="toc-container"
data-testid="toc-scroll-container"
className={tcls(className)}
style={style}
>
Expand Down
1 change: 1 addition & 0 deletions packages/gitbook/src/components/TableOfContents/TableOfContents.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export function TableOfContents(props: {

return (
<aside // Sidebar container, responsible for setting the right dimensions and position for the sidebar.
data-testid="table-of-contents"
className={tcls(
'group',
'page-no-toc:hidden',
Expand Down
17 changes: 11 additions & 6 deletions packages/react-math/src/MathJaX.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export default function MathJaXFormula(props: MathJaXFormulaProps) {
React.use(loadMathJaxScript(mathJaxUrl));
const [html, setHTML] = React.useState('');

const containerRef = React.useRef<HTMLDivElement | HTMLSpanElement>(null);
const containerRef = React.useRef<HTMLDivElement>(null);

// Typeset the formula
React.useEffect(() => {
Expand All @@ -48,11 +48,16 @@ export default function MathJaXFormula(props: MathJaXFormulaProps) {
};
}, [inline, formula]);

return React.createElement(inline ? 'span' : 'div', {
className,
ref: containerRef,
dangerouslySetInnerHTML: { __html: html },
});
const Component = inline ? 'span' : 'div';

return (
<Component
ref={containerRef}
className={className}
aria-busy={!html ? true : undefined}
dangerouslySetInnerHTML={{ __html: html }}
/>
);
}

let mathJaxPromise: Promise<void> | null = null;
Expand Down

0 comments on commit e000f23

Please sign in to comment.