Add testing for electron app using windows github action runner (#5421)

Author: cmdcolin

.github/workflows/windows-electron.yml

@@ -0,0 +1,34 @@
+name: Windows Electron Tests
+
+on:
+  push:
+    branches:
+      - main
+
+permissions:
+  contents: read
+
+jobs:
+  windows-electron-e2e:
+    name: Windows Electron E2E Tests
+    runs-on: windows-latest
+    steps:
+      - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
+        with:
+          persist-credentials: false
+      - uses: pnpm/action-setup@v4
+        with:
+          version: 10
+      - uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f # v6.1.0
+        with:
+          node-version: '22'
+          cache: 'pnpm'
+
+      - name: Install deps
+        run: pnpm install --frozen-lockfile
+
+      - name: Build and test Electron on Windows
+        run: pnpm test:e2e:ci:win
+        working-directory: products/jbrowse-desktop
+        env:
+          NO_MINIMIZE: true

packages/core/src/util/tracks.ts

@@ -181,12 +181,24 @@ export function getFileName(track: FileLocation) {
   const uri = 'uri' in track ? track.uri : undefined
   const localPath = 'localPath' in track ? track.localPath : undefined
   const blob = 'blobId' in track ? track : undefined
-  return (
-    blob?.name ||
-    uri?.slice(uri.lastIndexOf('/') + 1) ||
-    localPath?.slice(localPath.replace(/\\/g, '/').lastIndexOf('/') + 1) ||
-    ''
-  )
+
+  if (blob?.name) {
+    return blob.name
+  }
+
+  if (uri) {
+    // Normalize path separators and find the last one
+    // This handles both forward slashes and Windows backslashes in file:// URLs
+    const normalized = uri.replace(/\\/g, '/')
+    return normalized.slice(normalized.lastIndexOf('/') + 1)
+  }
+
+  if (localPath) {
+    const normalized = localPath.replace(/\\/g, '/')
+    return normalized.slice(normalized.lastIndexOf('/') + 1)
+  }
+
+  return ''
 }
 
 export function guessAdapter(

packages/text-indexing/src/TextIndexing.ts

@@ -7,7 +7,7 @@ import { checkStopToken } from '@jbrowse/core/util/stopToken'
 import { ixIxxStream } from 'ixixx'
 
 // misc
-import { generateMeta } from './types/common.ts'
+import { generateMeta, sanitizeForFilename } from './types/common.ts'
 import { indexGff3 } from './types/gff3Adapter.ts'
 import { indexVcf } from './types/vcfAdapter.ts'
 
@@ -290,7 +290,8 @@ function getLoc(attr: string, config: Track) {
 }
 
 function runIxIxx(readStream: Readable, idxLocation: string, name: string) {
-  const ixFilename = path.join(idxLocation, 'trix', `${name}.ix`)
-  const ixxFilename = path.join(idxLocation, 'trix', `${name}.ixx`)
+  const safeName = sanitizeForFilename(name)
+  const ixFilename = path.join(idxLocation, 'trix', `${safeName}.ix`)
+  const ixxFilename = path.join(idxLocation, 'trix', `${safeName}.ixx`)
   return ixIxxStream(readStream, ixFilename, ixxFilename)
 }

packages/text-indexing/src/types/common.ts

@@ -1,5 +1,6 @@
 import fs from 'fs'
 import path from 'path'
+import { fileURLToPath } from 'url'
 
 import fetch from 'node-fetch'
 
@@ -31,12 +32,12 @@ export function isURL(FileName: string) {
   return url.protocol === 'http:' || url.protocol === 'https:'
 }
 
-// Convert file:// URLs to local paths
-function fileUrlToPath(fileUrl: string): string | undefined {
+// Convert file:// URLs to local paths (handles Windows paths correctly)
+function convertFileUrlToPath(fileUrl: string): string | undefined {
   try {
     const url = new URL(fileUrl)
     if (url.protocol === 'file:') {
-      return url.pathname
+      return fileURLToPath(url)
     }
   } catch {
     // not a valid URL
@@ -66,7 +67,7 @@ export async function getLocalOrRemoteStream({
     return result.body
   } else {
     // Handle file:// URLs by converting to local path
-    const localPath = fileUrlToPath(file) ?? file
+    const localPath = convertFileUrlToPath(file) ?? file
     const filename = path.isAbsolute(localPath)
       ? localPath
       : path.join(out, localPath)
@@ -152,6 +153,12 @@ export function guessAdapterFromFileName(filePath: string): Track {
   }
 }
 
+// Sanitize a string to be safe for use in filenames on all platforms
+// Replaces characters that are invalid in Windows filenames: \ / : * ? " < > |
+export function sanitizeForFilename(name: string) {
+  return name.replace(/[\\/:*?"<>|]/g, '_')
+}
+
 /**
  * Generates metadata of index given a filename (trackId or assembly)
  */
@@ -170,8 +177,9 @@ export async function generateMeta({
   featureTypesToExclude: string[]
   assemblyNames: string[]
 }) {
+  const safeName = sanitizeForFilename(name)
   fs.writeFileSync(
-    path.join(outDir, 'trix', `${name}_meta.json`),
+    path.join(outDir, 'trix', `${safeName}_meta.json`),
     JSON.stringify(
       {
         dateCreated: new Date().toISOString(),

pnpm-lock.yaml

@@ -19,14 +19,28 @@ export function setupAutoUpdater(
   autoUpdater.autoDownload = false
 
   autoUpdater.on('error', (error: Error) => {
+    const errorMessage = `Error in auto-updater: ${error}`
+    sendStatusToWindow(getMainWindow(), errorMessage)
+
+    // Skip dialogs in CI environments to avoid blocking tests
+    if (process.env.CI) {
+      console.error('Auto-updater error (CI mode, skipping dialog):', error)
+      return
+    }
     dialog.showErrorBox(
-      'Error: ',
+      'Update Error',
       // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
       error == null ? 'unknown' : (error.stack || error).toString(),
     )
   })
 
   autoUpdater.on('update-available', async () => {
+    // Skip dialogs in CI environments
+    if (process.env.CI) {
+      // eslint-disable-next-line no-console
+      console.log('Update available (CI mode, skipping dialog)')
+      return
+    }
     const result = await dialog.showMessageBox({
       type: 'info',
       title: 'Found updates',
@@ -46,11 +60,13 @@ export function setupAutoUpdater(
     sendStatusToWindow(getMainWindow(), 'Checking for update...')
   })
 
-  autoUpdater.on('error', (err: Error) => {
-    sendStatusToWindow(getMainWindow(), `Error in auto-updater: ${err}`)
-  })
-
   autoUpdater.on('update-downloaded', () => {
+    // Skip dialogs in CI environments
+    if (process.env.CI) {
+      // eslint-disable-next-line no-console
+      console.log('Update downloaded (CI mode, skipping dialog)')
+      return
+    }
     dialog
       .showMessageBox({
         type: 'info',

products/jbrowse-desktop/electron/autoUpdater.ts

@@ -1,4 +1,4 @@
-import { app, ipcMain } from 'electron'
+import { app, dialog, ipcMain } from 'electron'
 import contextMenu from 'electron-context-menu'
 import debug from 'electron-debug'
 import pkg from 'electron-updater'
@@ -11,6 +11,7 @@ import { registerSessionHandlers } from './ipc/sessionHandlers.ts'
 import { initializePaths } from './paths.ts'
 import { createAuthWindow, createMainWindow } from './window.ts'
 
+import type { AppPaths } from './paths.ts'
 import type { BrowserWindow } from 'electron'
 
 const { autoUpdater } = pkg
@@ -22,31 +23,41 @@ debug({ showDevTools: false, isEnabled: true })
 // Environment variables
 const DEV_SERVER_URL = process.env.DEV_SERVER_URL
 
-// Initialize paths once
-const paths = initializePaths()
-
 // Main window reference
 let mainWindow: BrowserWindow | null = null
 
+// Paths - initialized after app is ready
+let paths: AppPaths
+
 function getMainWindow() {
   return mainWindow
 }
 
+/**
+ * Shows an error dialog to the user and optionally quits the app
+ */
+function showFatalError(title: string, error: unknown, shouldQuit = true) {
+  const message = error instanceof Error ? error.message : String(error)
+  const detail = error instanceof Error ? error.stack : undefined
+  console.error(`${title}:`, error)
+
+  dialog.showErrorBox(title, detail ? `${message}\n\n${detail}` : message)
+
+  if (shouldQuit) {
+    app.quit()
+  }
+}
+
 /**
  * Creates the main window and initializes the application
  */
 async function initialize() {
-  try {
-    await initializeFileSystem(paths)
-    mainWindow = await createMainWindow(autoUpdater, DEV_SERVER_URL)
+  await initializeFileSystem(paths)
+  mainWindow = await createMainWindow(autoUpdater, DEV_SERVER_URL)
 
-    mainWindow.on('closed', () => {
-      mainWindow = null
-    })
-  } catch (error) {
-    console.error('Failed to initialize application:', error)
-    throw error
-  }
+  mainWindow.on('closed', () => {
+    mainWindow = null
+  })
 }
 
 /**
@@ -72,15 +83,19 @@ function registerIpcHandlers() {
   )
 }
 
-// Setup auto-updater
+// Setup auto-updater (just registers event listeners, safe before ready)
 setupAutoUpdater(autoUpdater, getMainWindow)
 
-// Register IPC handlers
-registerIpcHandlers()
-
 // App lifecycle handlers
 app.on('ready', async () => {
-  await initialize()
+  try {
+    // Initialize paths after app is ready to ensure app.getPath() works reliably
+    paths = initializePaths()
+    registerIpcHandlers()
+    await initialize()
+  } catch (error) {
+    showFatalError('Failed to initialize application', error)
+  }
 })
 
 app.on('window-all-closed', () => {
@@ -91,6 +106,10 @@ app.on('window-all-closed', () => {
 
 app.on('activate', async () => {
   if (mainWindow === null) {
-    await initialize()
+    try {
+      await initialize()
+    } catch (error) {
+      showFatalError('Failed to create window', error)
+    }
   }
 })

products/jbrowse-desktop/electron/electron.ts

@@ -1,5 +1,5 @@
 import path from 'path'
-import url from 'url'
+import url, { pathToFileURL } from 'url'
 
 import electron, { BrowserWindow, Menu, app, shell } from 'electron'
 import windowStateKeeper from 'electron-window-state'
@@ -15,9 +15,7 @@ const DEFAULT_DEV_SERVER_URL = 'http://localhost:3000'
 
 function getAppUrl(devServerUrl: URL): URL {
   if (app.isPackaged) {
-    return new URL(
-      `file://${path.join(app.getAppPath(), 'build', 'index.html')}`,
-    )
+    return pathToFileURL(path.join(app.getAppPath(), 'build', 'index.html'))
   }
   return devServerUrl
 }
@@ -84,11 +82,14 @@ export async function createMainWindow(
   mainWindowState.manage(mainWindow)
 
   // This ready-to-show handler must be attached before the loadURL
-  mainWindow.once('ready-to-show', () => {
-    autoUpdater.checkForUpdatesAndNotify().catch((e: unknown) => {
-      console.error(e)
+  // Skip auto-update check in CI environments to avoid blocking dialogs
+  if (!process.env.CI) {
+    mainWindow.once('ready-to-show', () => {
+      autoUpdater.checkForUpdatesAndNotify().catch((e: unknown) => {
+        console.error(e)
+      })
     })
-  })
+  }
 
   const serverUrl = new URL(devServerUrl || DEFAULT_DEV_SERVER_URL)
   const appUrl = getAppUrl(serverUrl)