Skip to content
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

Fix visual testing flakiness #2750

Merged
merged 8 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 87 additions & 20 deletions packages/gitbook/e2e/pages.spec.ts
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
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
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
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
19 changes: 12 additions & 7 deletions packages/react-math/src/MathJaX.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ export default function MathJaXFormula(props: MathJaXFormulaProps) {
const { formula, inline, className, mathJaxUrl } = props;

// @ts-ignore - React.use doesn't seem define in typing
React.use(loadMathJaxScript(mathJaxUrl));
// React.use(loadMathJaxScript(mathJaxUrl));
gregberge marked this conversation as resolved.
Show resolved Hide resolved
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
Loading