refactor(erp-auth): implement precise login result detection with three outcomes
- Add waitForLoginResult() method using Promise.race to detect: - Success: .nc-workbench-icon element visible - Failure: '名称或密码错误' error text visible - Force login: click confirm button and re-detect - Extract timeout constants (PAGE_LOAD_TIMEOUT, LOGIN_RESULT_TIMEOUT, FORCE_LOGIN_TIMEOUT) - Improve error handling with clear error messages - Add unit tests for class structure verification - Fix test setup for Electron app mock Fixes: ERP login success/failure detection was ambiguous
This commit is contained in:
14
package-lock.json
generated
14
package-lock.json
generated
@@ -41,6 +41,7 @@
|
|||||||
"@vitejs/plugin-react": "^5.1.1",
|
"@vitejs/plugin-react": "^5.1.1",
|
||||||
"@vitest/coverage-v8": "^4.0.18",
|
"@vitest/coverage-v8": "^4.0.18",
|
||||||
"autoprefixer": "^10.4.27",
|
"autoprefixer": "^10.4.27",
|
||||||
|
"dotenv": "^17.3.1",
|
||||||
"electron": "^39.2.6",
|
"electron": "^39.2.6",
|
||||||
"electron-builder": "^26.0.12",
|
"electron-builder": "^26.0.12",
|
||||||
"electron-vite": "^5.0.0",
|
"electron-vite": "^5.0.0",
|
||||||
@@ -5591,6 +5592,19 @@
|
|||||||
"node": ">=0.10.0"
|
"node": ">=0.10.0"
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
"node_modules/dotenv": {
|
||||||
|
"version": "17.3.1",
|
||||||
|
"resolved": "https://registry.npmjs.org/dotenv/-/dotenv-17.3.1.tgz",
|
||||||
|
"integrity": "sha512-IO8C/dzEb6O3F9/twg6ZLXz164a2fhTnEWb95H23Dm4OuN+92NmEAlTrupP9VW6Jm3sO26tQlqyvyi4CsnY9GA==",
|
||||||
|
"dev": true,
|
||||||
|
"license": "BSD-2-Clause",
|
||||||
|
"engines": {
|
||||||
|
"node": ">=12"
|
||||||
|
},
|
||||||
|
"funding": {
|
||||||
|
"url": "https://dotenvx.com"
|
||||||
|
}
|
||||||
|
},
|
||||||
"node_modules/dotenv-expand": {
|
"node_modules/dotenv-expand": {
|
||||||
"version": "11.0.7",
|
"version": "11.0.7",
|
||||||
"resolved": "https://registry.npmjs.org/dotenv-expand/-/dotenv-expand-11.0.7.tgz",
|
"resolved": "https://registry.npmjs.org/dotenv-expand/-/dotenv-expand-11.0.7.tgz",
|
||||||
|
|||||||
@@ -61,6 +61,7 @@
|
|||||||
"@vitejs/plugin-react": "^5.1.1",
|
"@vitejs/plugin-react": "^5.1.1",
|
||||||
"@vitest/coverage-v8": "^4.0.18",
|
"@vitest/coverage-v8": "^4.0.18",
|
||||||
"autoprefixer": "^10.4.27",
|
"autoprefixer": "^10.4.27",
|
||||||
|
"dotenv": "^17.3.1",
|
||||||
"electron": "^39.2.6",
|
"electron": "^39.2.6",
|
||||||
"electron-builder": "^26.0.12",
|
"electron-builder": "^26.0.12",
|
||||||
"electron-vite": "^5.0.0",
|
"electron-vite": "^5.0.0",
|
||||||
@@ -74,9 +75,9 @@
|
|||||||
"react": "^19.2.1",
|
"react": "^19.2.1",
|
||||||
"react-dom": "^19.2.1",
|
"react-dom": "^19.2.1",
|
||||||
"tailwindcss": "^4.2.1",
|
"tailwindcss": "^4.2.1",
|
||||||
|
"tsx": "^4.19.3",
|
||||||
"typescript": "^5.9.3",
|
"typescript": "^5.9.3",
|
||||||
"vite": "^7.2.6",
|
"vite": "^7.2.6",
|
||||||
"vitest": "^4.0.18",
|
"vitest": "^4.0.18"
|
||||||
"tsx": "^4.19.3"
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,6 +4,11 @@ import { createLogger } from '../logger'
|
|||||||
|
|
||||||
const log = createLogger('ErpAuthService')
|
const log = createLogger('ErpAuthService')
|
||||||
|
|
||||||
|
// Timeout constants
|
||||||
|
const PAGE_LOAD_TIMEOUT = 10000
|
||||||
|
const LOGIN_RESULT_TIMEOUT = 15000
|
||||||
|
const FORCE_LOGIN_TIMEOUT = 5000
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* ERP Authentication Service
|
* ERP Authentication Service
|
||||||
* Manages login session and browser lifecycle
|
* Manages login session and browser lifecycle
|
||||||
@@ -51,10 +56,13 @@ export class ErpAuthService {
|
|||||||
await page.goto(loginUrl)
|
await page.goto(loginUrl)
|
||||||
|
|
||||||
// Wait for page to load
|
// Wait for page to load
|
||||||
await page.waitForLoadState('domcontentloaded', { timeout: 10000 })
|
await page.waitForLoadState('domcontentloaded', { timeout: PAGE_LOAD_TIMEOUT })
|
||||||
|
|
||||||
// Wait for iframe to be present
|
// Wait for iframe to be present
|
||||||
await page.waitForSelector('#forwardFrame', { state: 'attached', timeout: 15000 })
|
await page.waitForSelector('#forwardFrame', {
|
||||||
|
state: 'attached',
|
||||||
|
timeout: LOGIN_RESULT_TIMEOUT
|
||||||
|
})
|
||||||
|
|
||||||
// Extract forwardFrame (Python: main_frame = page.locator("#forwardFrame").content_frame)
|
// Extract forwardFrame (Python: main_frame = page.locator("#forwardFrame").content_frame)
|
||||||
// This is the main working frame for all subsequent operations
|
// This is the main working frame for all subsequent operations
|
||||||
@@ -89,7 +97,7 @@ export class ErpAuthService {
|
|||||||
throw new Error(`Failed to click login button: ${e}`)
|
throw new Error(`Failed to click login button: ${e}`)
|
||||||
}
|
}
|
||||||
|
|
||||||
await page.waitForLoadState('domcontentloaded', { timeout: 10000 }).catch(() => {
|
await page.waitForLoadState('domcontentloaded', { timeout: PAGE_LOAD_TIMEOUT }).catch(() => {
|
||||||
log.warn('Page load state check timed out, continuing')
|
log.warn('Page load state check timed out, continuing')
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -118,9 +126,11 @@ export class ErpAuthService {
|
|||||||
|
|
||||||
try {
|
try {
|
||||||
await Promise.race([
|
await Promise.race([
|
||||||
successLocator.waitFor({ state: 'visible', timeout: 15000 }),
|
successLocator.waitFor({ state: 'visible', timeout: LOGIN_RESULT_TIMEOUT }),
|
||||||
errorLocator.waitFor({ state: 'visible', timeout: 15000 }),
|
errorLocator.waitFor({ state: 'visible', timeout: LOGIN_RESULT_TIMEOUT }),
|
||||||
forceLoginButton.waitFor({ state: 'visible', timeout: 5000 }).then(async () => {
|
forceLoginButton
|
||||||
|
.waitFor({ state: 'visible', timeout: FORCE_LOGIN_TIMEOUT })
|
||||||
|
.then(async () => {
|
||||||
log.info('Force login dialog detected, clicking confirm')
|
log.info('Force login dialog detected, clicking confirm')
|
||||||
await forceLoginButton.click()
|
await forceLoginButton.click()
|
||||||
await this.waitForLoginResult(mainFrame)
|
await this.waitForLoginResult(mainFrame)
|
||||||
|
|||||||
@@ -1,7 +1,17 @@
|
|||||||
import { beforeAll, afterAll } from 'vitest'
|
import { beforeAll, afterAll, vi } from 'vitest'
|
||||||
import dotenv from 'dotenv'
|
import dotenv from 'dotenv'
|
||||||
import path from 'path'
|
import path from 'path'
|
||||||
|
|
||||||
|
// Mock electron app module for unit tests
|
||||||
|
vi.mock('electron', () => ({
|
||||||
|
app: {
|
||||||
|
isPackaged: false,
|
||||||
|
isReady: vi.fn().mockReturnValue(false),
|
||||||
|
getPath: vi.fn().mockReturnValue(path.join(process.cwd(), 'logs')),
|
||||||
|
on: vi.fn()
|
||||||
|
}
|
||||||
|
}))
|
||||||
|
|
||||||
// Load environment variables from project root
|
// Load environment variables from project root
|
||||||
dotenv.config({ path: path.resolve(process.cwd(), '.env') })
|
dotenv.config({ path: path.resolve(process.cwd(), '.env') })
|
||||||
|
|
||||||
|
|||||||
@@ -56,4 +56,38 @@ describe('ERP Authentication Service (Unit)', () => {
|
|||||||
await expect(service.close()).resolves.toBeUndefined()
|
await expect(service.close()).resolves.toBeUndefined()
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe('Class Structure', () => {
|
||||||
|
let service: ErpAuthService
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
const config: ErpConfig = {
|
||||||
|
url: 'https://test.example.com',
|
||||||
|
username: 'testuser',
|
||||||
|
password: 'testpass'
|
||||||
|
}
|
||||||
|
service = new ErpAuthService(config)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('should have login method that returns a Promise', () => {
|
||||||
|
expect(service.login).toBeDefined()
|
||||||
|
expect(typeof service.login).toBe('function')
|
||||||
|
expect(service.login()).toBeInstanceOf(Promise)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('should have close method', () => {
|
||||||
|
expect(service.close).toBeDefined()
|
||||||
|
expect(typeof service.close).toBe('function')
|
||||||
|
})
|
||||||
|
|
||||||
|
it('should have getSession method', () => {
|
||||||
|
expect(service.getSession).toBeDefined()
|
||||||
|
expect(typeof service.getSession).toBe('function')
|
||||||
|
})
|
||||||
|
|
||||||
|
it('should have isActive method', () => {
|
||||||
|
expect(service.isActive).toBeDefined()
|
||||||
|
expect(typeof service.isActive).toBe('function')
|
||||||
|
})
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user