Fix existing tests

marp-team · Sep 26, 2024 · 624a83d · 624a83d
1 parent 8a1e878
commit 624a83d
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 20 deletions.
diff --git a/src/browser/browser.ts b/src/browser/browser.ts
@@ -76,6 +76,9 @@ export abstract class Browser
     const puppeteer = await this.launch()
     const page = await puppeteer.newPage()
 
+    page.setDefaultTimeout(this.timeout)
+    page.setDefaultNavigationTimeout(this.timeout)
+
     try {
       return await fn(page)
     } finally {

diff --git a/src/browser/manager.ts b/src/browser/manager.ts
@@ -35,6 +35,10 @@ export class BrowserManager implements AsyncDisposable {
     this.configure(config)
   }
 
+  get timeout() {
+    return this._timeout
+  }
+
   configure(config: BrowserManagerConfig) {
     if (config.finders) {
       this._finders = ([] as FinderName[]).concat(config.finders)
@@ -86,7 +90,7 @@ export class BrowserManager implements AsyncDisposable {
       debugBrowser('Use browser class for conversion: %o', browser)
 
       // @ts-expect-error ts2511: TS cannot create an instance of an abstract class
-      this._conversionBrowser = new browser({ path, timeout: this._timeout })
+      this._conversionBrowser = new browser({ path, timeout: this.timeout })
     }
     return this._conversionBrowser!
   }
@@ -103,7 +107,7 @@ export class BrowserManager implements AsyncDisposable {
 
       this._previewBrowser = new ChromeCdpBrowser({
         path,
-        timeout: this._timeout,
+        timeout: this.timeout,
       })
     }
     return this._previewBrowser

diff --git a/test/browser/finder.ts b/test/browser/finder.ts
@@ -2,7 +2,7 @@ import path from 'node:path'
 import { ChromeBrowser } from '../../src/browser/browsers/chrome'
 import { ChromeCdpBrowser } from '../../src/browser/browsers/chrome-cdp'
 import { FirefoxBrowser } from '../../src/browser/browsers/firefox'
-import { autoFinders, findBrowser } from '../../src/browser/finder'
+import { defaultFinders, findBrowser } from '../../src/browser/finder'
 import { CLIError } from '../../src/error'
 
 afterEach(() => {
@@ -148,7 +148,7 @@ describe('Browser finder', () => {
       })
 
       it('normalizes executable path if preferred path was pointed to valid app bundle', async () => {
-        const browser = await findBrowser(autoFinders, {
+        const browser = await findBrowser(defaultFinders, {
           preferredPath: macBundle('Valid.app'),
         })
 
@@ -159,7 +159,7 @@ describe('Browser finder', () => {
       })
 
       it('does not normalize executable path if preferred path was pointed to invalid app bundle', async () => {
-        const browser = await findBrowser(autoFinders, {
+        const browser = await findBrowser(defaultFinders, {
           preferredPath: macBundle('Invalid.app'),
         })
 

diff --git a/test/converter.ts b/test/converter.ts
@@ -18,8 +18,6 @@ import { bare as bareTpl } from '../src/templates'
 import { ThemeSet } from '../src/theme'
 import { WatchNotifier } from '../src/watcher'
 
-const { CdpPage } = require('puppeteer-core/lib/cjs/puppeteer/cdp/Page') // eslint-disable-line @typescript-eslint/no-require-imports -- Puppeteer's internal module
-
 const timeout = 60000
 
 let mkdirSpy: jest.SpiedFunction<typeof fs.promises.mkdir>
@@ -974,15 +972,18 @@ describe('Converter', () => {
       it(
         'converts markdown file into PPTX',
         async () => {
-          const setViewport = jest.spyOn(CdpPage.prototype, 'setViewport')
+          const cvt = converter()
+          const imageSpy = jest.spyOn(cvt as any, 'convertFileToImage')
 
-          await converter().convertFile(new File(onePath))
+          await cvt.convertFile(new File(onePath))
           expect(write).toHaveBeenCalled()
           expect(write.mock.calls[0][0]).toBe('test.pptx')
 
           // It has a different default scale x2
-          expect(setViewport).toHaveBeenCalledWith(
-            expect.objectContaining({ deviceScaleFactor: 2 })
+          expect(imageSpy).toHaveBeenLastCalledWith(
+            expect.anything(), // Template
+            expect.anything(), // File
+            expect.objectContaining({ scale: 2 })
           )
 
           // ZIP PK header for Office Open XML
@@ -1001,16 +1002,17 @@ describe('Converter', () => {
         it(
           'assigns meta info thorugh PptxGenJs',
           async () => {
-            const setViewport = jest.spyOn(CdpPage.prototype, 'setViewport')
-
-            await converter({
+            const cvt = converter({
               imageScale: 1,
               globalDirectives: {
                 title: 'Test meta',
                 description: 'Test description',
                 author: 'author',
               },
-            }).convertFile(new File(onePath))
+            })
+            const imageSpy = jest.spyOn(cvt as any, 'convertFileToImage')
+
+            await cvt.convertFile(new File(onePath))
 
             const pptx: Buffer = write.mock.calls[0][1]
             const meta = await getPptxDocProps(pptx)
@@ -1020,8 +1022,10 @@ describe('Converter', () => {
             expect(meta['dc:creator']).toBe('author')
 
             // Custom scale
-            expect(setViewport).toHaveBeenCalledWith(
-              expect.objectContaining({ deviceScaleFactor: 1 })
+            expect(imageSpy).toHaveBeenLastCalledWith(
+              expect.anything(), // Template
+              expect.anything(), // File
+              expect.objectContaining({ scale: 1 })
             )
           },
           timeout

diff --git a/test/marp-cli.ts b/test/marp-cli.ts
@@ -38,8 +38,8 @@ const runForObservation = async (argv: string[]) => {
   return ret
 }
 
-jest.mock('cosmiconfig')
 jest.mock('node:fs', () => require('./__mocks__/node/fs')) // eslint-disable-line @typescript-eslint/no-require-imports, jest/no-mocks-import -- Windows file system cannot use `:`
+jest.mock('cosmiconfig')
 jest.mock('../src/preview')
 jest.mock('../src/watcher', () => jest.createMockFromModule('../src/watcher'))
 
@@ -1344,15 +1344,15 @@ describe('Marp CLI', () => {
 
       it('follows specified timeout in conversion that is using Puppeteer', async () => {
         expect(
-          (await conversion(onePath, '--pdf')).options.puppeteerTimeout
+          (await conversion(onePath, '--pdf')).options.browserManager.timeout
         ).toBe(12345)
       })
 
       it('does not follows specified timeout if the env value is not valid number', async () => {
         process.env.PUPPETEER_TIMEOUT = 'invalid'
 
         expect(
-          (await conversion(onePath, '--pdf')).options.puppeteerTimeout
+          (await conversion(onePath, '--pdf')).options.browserManager.timeout
         ).toBeUndefined()
       })
     })

diff --git a/test/server.ts b/test/server.ts
@@ -4,6 +4,7 @@ import { Marp } from '@marp-team/marp-core'
 import { load } from 'cheerio'
 import express, { type Express } from 'express'
 import request from 'supertest'
+import { BrowserManager } from '../src/browser/manager'
 import {
   Converter,
   ConvertType,
@@ -17,11 +18,16 @@ import { ThemeSet } from '../src/theme'
 jest.mock('express')
 
 describe('Server', () => {
+  let browserManager: BrowserManager
   let themeSet: ThemeSet
+
+  beforeAll(() => (browserManager = new BrowserManager()))
   beforeEach(async () => (themeSet = await ThemeSet.initialize([])))
+  afterAll(async () => browserManager?.dispose())
 
   const converter = (opts: Partial<ConverterOption> = {}) =>
     new Converter({
+      browserManager,
       themeSet,
       allowLocalFiles: false,
       engine: Marp,