-
Notifications
You must be signed in to change notification settings - Fork 1
Fix book scan errors and flaky tests #2628
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
base: main
Are you sure you want to change the base?
Conversation
…they are easier to fix
… with list with invalid start attribute
script/domVisitor.ts
Outdated
| linkElt.click(); | ||
| }, linkSelector); | ||
| await page.waitForSelector(`li[aria-label="Current Page"] ${linkSelector}`); | ||
| await page.waitForSelector(`${linkSelector}[aria-label="Current Page"]`); |
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 is due to a change in the ToC structure
| function configurePage(page: puppeteer.Page): ObservePageErrors { | ||
| let errorObserver: PageErrorObserver = () => null; | ||
| const cache = new QuickLRU<string, puppeteer.RespondOptions>({maxSize: cacheMaxSize}); | ||
| const cache = new QuickLRU<string, puppeteer.ResponseForRequest>({maxSize: cacheMaxSize}); |
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 is due to a puppeteer version update
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.
Pull request overview
This PR addresses book scan errors and flaky tests by updating Puppeteer to v13.7.0 (matching Lighthouse's expected version), fixing ToC structure changes, resolving test flakiness related to empty img src attributes, adding missing act() calls to tests, and allowing tolerance in scroll position tests to account for OS-specific rendering differences.
Key Changes
- Updated Puppeteer from v5 to v13.7.0 along with related testing dependencies (jest-puppeteer v5 to v6)
- Fixed test flakiness by properly wrapping state updates in
act()calls and handling async operations - Added tolerance for scroll position assertions to handle OS/platform differences in rendering
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Updated puppeteer to v13.7.0 and related dependencies (jest-puppeteer, axios, joi, etc.) |
| src/test/setup.ts | Fixed console.error and console.warn to accept multiple arguments using rest parameters |
| src/test/mocks/archiveLoader.ts | Added empty string resource for /resources/hash to fix img with empty src errors |
| src/test/browserutils.ts | Added getScrollTop helper function and updated type declarations for Puppeteer v13 compatibility |
| src/app/domUtils.spec.ts | Changed jest.runAllTimers() to jest.runOnlyPendingTimers() to avoid infinite loops |
| src/app/content/highlights/components/HighlightsPopUp.spec.tsx | Fixed missing renderer.act() calls and added async handling for promises |
| src/app/content/highlights/components/Highlights.spec.tsx | Added console.error spy to suppress expected error messages in tests |
| src/app/content/components/Page/PageToasts.spec.tsx | Wrapped component creation in renderer.act() and properly handled fake timers |
| src/app/content/components/Page.spec.tsx | Wrapped state updates in ReactTestUtils.act() and fixed typo (pageWithRefereces → pageWithReferences) |
| src/app/content/components/Content.browserspec.tsx | Added scroll position tolerance using getScrollTop helper and MAX_SCROLL_DIFF constant |
| script/domVisitor.ts | Fixed ToC selector and updated type from RespondOptions to ResponseForRequest |
| package.json | Updated puppeteer to ^13.7.0, jest-puppeteer to ^6, removed @types/puppeteer, updated yargs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
https://openstax.atlassian.net/browse/CORE-347