Skip to content

fixed integration tests to run on github actions #196

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.

Sign up for GitHub

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

Open
jasir99 wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

fixed integration tests to run on github actions #196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
51 changes: 35 additions & 16 deletions .github/workflows/e2e-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,7 @@ on:

jobs:
e2e-tests:
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]

runs-on: ${{ matrix.os }}
runs-on: ubuntu-latest

timeout-minutes: 30

Expand All @@ -29,24 +24,44 @@ jobs:
node-version: '20'
cache: 'npm'

- name: Verify Node.js version
run: |
node --version
npm --version

- name: Install dependencies
run: npm ci

- name: Build application
- name: Update browserslist database
run: npx update-browserslist-db@latest

- name: Build main process
run: npm run prestart
env:
NODE_ENV: development

# Linux-specific: Setup virtual display
- name: Setup display (Linux)
if: runner.os == 'Linux'
- name: Build renderer process
run: |
echo "Building renderer process..."
npm run build:renderer 2>&1 | tee build-output.log || (cat build-output.log && exit 1)
env:
NODE_ENV: production
# No code signing needed for test builds
SKIP_NOTARIZATION: 'true'

Comment on lines +43 to +51
Copy link

@coderabbitai coderabbitai bot Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/e2e-tests.yml | sed -n '35,55p'

Repository: rosettadb/dbt-studio

Length of output: 882

🏁 Script executed:

#!/bin/bash
# Test to confirm pipeline exit code behavior

echo "=== Testing pipeline exit code WITHOUT pipefail ==="
set +o pipefail  # Ensure pipefail is off
(
  false | tee /tmp/test.log
  exit_code=$?
  echo "Exit code: $exit_code"
)

echo ""
echo "=== Testing pipeline exit code WITH pipefail ==="
set -o pipefail  # Enable pipefail
(
  false | tee /tmp/test.log || true
  exit_code=$?
  echo "Exit code: $exit_code"
)

echo ""
echo "=== Testing current logic: (false | tee) || echo 'catch block' ==="
set +o pipefail  # Back to default
(
  false | tee /tmp/test.log || echo "CAUGHT FAILURE"
)

Repository: rosettadb/dbt-studio

Length of output: 421

🏁 Script executed:

#!/bin/bash
# Test pipeline exit code behavior using bash builtins

echo "=== Testing: (exit 1) | (exit 0) without pipefail ==="
set +o pipefail
(exit 1) | (exit 0)
echo "Pipeline exit code: $?"

echo ""
echo "=== Testing: (exit 1) | (exit 0) with pipefail ==="
set -o pipefail
(exit 1) | (exit 0)
echo "Pipeline exit code: $?"

echo ""
echo "=== Testing current GitHub Actions pattern ==="
set +o pipefail
(exit 1) | (exit 0) || echo "CAUGHT VIA || CLAUSE"

Repository: rosettadb/dbt-studio

Length of output: 261

Add set -euo pipefail to properly fail on build errors.

The pipeline without pipefail returns tee's exit code (0), masking npm failures. The current || (cat build-output.log && exit 1) clause never executes because the pipeline always succeeds from bash's perspective. Enabling pipefail ensures the pipeline returns the exit code of the first failing command.

Suggested fix 
       - name: Build renderer process
         run: |
+          set -euo pipefail
           echo "Building renderer process..."
-          npm run build:renderer 2>&1 | tee build-output.log || (cat build-output.log && exit 1)
+          npm run build:renderer 2>&1 | tee build-output.log
         env:
           NODE_ENV: production
           # No code signing needed for test builds
           SKIP_NOTARIZATION: 'true'
🤖 Prompt for AI Agents 
In @.github/workflows/e2e-tests.yml around lines 43 - 51, In the "Build renderer
process" GitHub Actions step (the run block that calls npm run build:renderer
and pipes to tee), prepend a strict shell setting by adding "set -euo pipefail"
at the top of the run script so the pipeline returns the failing command's exit
code (this makes the existing "|| (cat build-output.log && exit 1)" behave as
intended and causes the job to fail on npm errors).

- name: Copy renderer artifacts
run: |
mkdir -p .erb/renderer
cp -R release/app/dist/renderer/* .erb/renderer/

# Setup virtual display for headless testing
- name: Setup display
run: |
sudo apt-get update
sudo apt-get install -y xvfb libnss3 libatk1.0-0 libatk-bridge2.0-0 libcups2 libdrm2 libxkbcommon0 libxcomposite1 libxdamage1 libxfixes3 libxrandr2 libgbm1 libasound2
echo "DISPLAY=:99" >> $GITHUB_ENV

- name: Start Xvfb (Linux)
if: runner.os == 'Linux'
- name: Start Xvfb
run: |
Xvfb :99 -screen 0 1920x1080x24 &
sleep 3
Expand All @@ -63,12 +78,16 @@ jobs:
ELECTRON_NO_UPDATER: '1'
# Disable analytics in test
DISABLE_ANALYTICS: '1'
# Ensure headless operation
ELECTRON_DISABLE_GPU: '1'
# No code signing needed for test builds
SKIP_NOTARIZATION: 'true'

- name: Upload test results
uses: actions/upload-artifact@v4
if: always()
with:
name: test-results-${{ matrix.os }}
name: test-results
path: |
test-results/
playwright-report/
Expand All @@ -78,11 +97,11 @@ jobs:
uses: actions/upload-artifact@v4
if: failure()
with:
name: failure-screenshots-${{ matrix.os }}
name: failure-screenshots
path: test-results/screenshots/
retention-days: 7

# Summary job that requires all matrix jobs to pass
# Summary job
e2e-tests-summary:
needs: e2e-tests
runs-on: ubuntu-latest
Expand All @@ -91,7 +110,7 @@ jobs:
- name: Check E2E test results
run: |
if [ "${{ needs.e2e-tests.result }}" == "failure" ]; then
echo "E2E tests failed on one or more platforms"
echo "E2E tests failed"
exit 1
fi
echo "All E2E tests passed!"
24 changes: 7 additions & 17 deletions .github/workflows/integration-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,15 @@
name: Integration Tests

on:
push:
branches: [develop]
pull_request:
branches: [develop]
branches: [dev]

jobs:
integration-tests:
name: Integration Tests (Node ${{ matrix.node-version }})
runs-on: ${{ matrix.os }}
name: Integration Tests
runs-on: ubuntu-latest
timeout-minutes: 30

strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-latest]
node-version: [18, 20]

services:
postgres:
image: postgres:16
Expand All @@ -37,14 +29,13 @@ jobs:
- name: Checkout repository
uses: actions/checkout@v4

- name: Setup Node.js ${{ matrix.node-version }}
- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}
node-version: '20'
cache: 'npm'

- name: Install system dependencies (Ubuntu)
if: runner.os == 'Linux'
- name: Install system dependencies
run: |
sudo apt-get update
sudo apt-get install -y libsecret-1-dev
Expand All @@ -69,14 +60,13 @@ jobs:
if: always()
uses: actions/upload-artifact@v4
with:
name: integration-test-results-${{ matrix.os }}-node${{ matrix.node-version }}
name: integration-test-results
path: |
coverage/
test-results/
retention-days: 7

- name: Upload coverage to Codecov
if: matrix.os == 'ubuntu-latest' && matrix.node-version == '20'
uses: codecov/codecov-action@v4
with:
files: ./coverage/coverage-final.json
Expand Down
8 changes: 8 additions & 0 deletions e2e/fixtures/electron.fixture.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,17 @@ export const test = base.extend<ElectronFixtures>({
}

// Launch the Electron app
// In CI environments, Electron runs headless automatically via Playwright
const electronApp = await electron.launch({
args: [
mainBundlePath,
`--user-data-dir=${userData}`,
'--disable-gpu',
'--no-sandbox',
'--disable-dev-shm-usage',
// Additional CI-friendly flags
'--disable-software-rasterizer',
'--disable-extensions',
],
env: {
...process.env,
Expand All @@ -131,6 +135,10 @@ export const test = base.extend<ElectronFixtures>({
ELECTRON_NO_UPDATER: '1',
// Disable analytics in tests
DISABLE_ANALYTICS: '1',
// Ensure headless operation in CI
...(process.env.CI === 'true' && {
ELECTRON_DISABLE_GPU: '1',
}),
},
});

Expand Down
9 changes: 9 additions & 0 deletions jest.integration.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,18 @@ module.exports = {
...baseConfig,
testMatch: ['**/tests/integration/**/*.test.ts'],
testEnvironment: 'node',
// Integration tests don't need the build check - they run in Node.js, not Electron
setupFiles: [],
setupFilesAfterEnv: ['<rootDir>/tests/setup/jest.integration.setup.ts'],
testTimeout: 30000,
maxWorkers: 1,
// Fix Jest Haste collision by ignoring release/app directory
modulePathIgnorePatterns: ['<rootDir>/release/app'],
testPathIgnorePatterns: [
...(baseConfig.testPathIgnorePatterns || []),
'release/app/dist',
'.erb/dll',
],
moduleNameMapper: {
...baseConfig.moduleNameMapper,
'^@services/(.*)$': '<rootDir>/src/main/services/$1',
Expand Down
Loading