Bug #12094 - Fix _appiumStart method (#12095)

* Allure Reporter Improvements. Adding support to fixtures for mocha/jasmine. Update the Cucumber Step Mode.

Change constants and option names.

* Fix test:depcheck in packages/wdio-allure-reporter/src/state.ts

* Fixes:
- using 'getErrorFromFailedTest' to format error messages

* Fixes:
- removing unnecessary code for mocha and jasmine

* Fixes:
- eslint and spaces before from.

* Fixes:
- Property 'labels' does not exist on type 'ExecutableItem'.
- error TS2835: Relative import paths need explicit

* Fixes:
- Allure Report Methods.
- Eslint Fixes
- Test Cases wdio-allure-reporter related

* Fixes:
- Fix undefined in fixtures

* Update packages/wdio-allure-reporter/tests/allureFeatures.test.ts

* Fixes:
- Fixes (base on comments)
- import check
- eslit command executed
- npm run test command executed
- no it.only in the commit

* Fixes:
- Option name rollback - comment left
- Option for customized the test error when hook fails (put off for next ticket)

* Fixes:
- bad merge from main (imports)

* Fixes:
- Separate hook and test status getter
- review of test cases for fixtures.
- fixtures cannot be failed the worst status could be broken.

Signed-off-by: Manuel Lara <lararojas.mr@gmail.com>

* Git #12094
The _startAppium is stucking whitout timeout when the promise emit 'data' but the data not includes the specific text.

I've set a timeout y default in 60sec by security, and implemented a validation to reject only once at the first failure.

Add some comments.

* Removing all allure changes. They were sending to other branch.

* Validating resolve in case that the stdio has non error messages, like debugger messages, etc. The message is log as warning, and the event is resolve.

* Removing semicolons from code.

* Resolving the promise

* unit tests for timeout new parameter

Signed-off-by: Lara, Manuel <lararojas.mr@gmail.com>

* fixes unit test - appium service

Signed-off-by: Lara, Manuel <lararojas.mr@gmail.com>

* Update packages/wdio-appium-service/src/launcher.ts

Co-authored-by: Christian Bromann <git@bromann.dev>

* Update packages/wdio-appium-service/src/launcher.ts - Type

Co-authored-by: Christian Bromann <git@bromann.dev>

* Create a const for timeout and set the timeout in 30 sec by default.

Signed-off-by: Lara, Manuel <lararojas.mr@gmail.com>

* Fix eslint errors.

Signed-off-by: Lara, Manuel <lararojas.mr@gmail.com>

* Fix test cases, changes in the method to cover all missing cases.

Signed-off-by: Lara, Manuel <lararojas.mr@gmail.com>

---------

Signed-off-by: Manuel Lara <lararojas.mr@gmail.com>
Signed-off-by: Lara, Manuel <lararojas.mr@gmail.com>
Co-authored-by: Christian Bromann <git@bromann.dev>
This commit is contained in:
Manuel Lara 2024-01-29 19:40:05 -03:00 committed by GitHub
parent 165d10947b
commit 91f261e7f8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 68 additions and 12 deletions

View file

@ -24,6 +24,7 @@ const DEFAULT_CONNECTION = {
hostname: '127.0.0.1',
path: '/'
}
const APPIUM_START_TIMEOUT = 30 * 1000
export default class AppiumLauncher implements Services.ServiceInstance {
private readonly _logPath?: string
@ -154,16 +155,45 @@ export default class AppiumLauncher implements Services.ServiceInstance {
this._process.kill()
}
}
private _startAppium(command: string, args: Array<string>) {
private _startAppium(command: string, args: Array<string>, timeout = APPIUM_START_TIMEOUT) {
log.info(`Will spawn Appium process: ${command} ${args.join(' ')}`)
const process: ChildProcessByStdio<null, Readable, Readable> = spawn(command, args, { stdio: ['ignore', 'pipe', 'pipe'] })
let error: Error | undefined
// just for validate the first error
let errorCaptured = false
// to set a timeout for the promise
let timeoutId: NodeJS.Timeout
// to store the first error message
let error: string
return new Promise<ChildProcessByStdio<null, Readable, Readable>>((resolve, reject) => {
let outputBuffer = ''
/**
* set timeout for promise. If Appium does not start within given timeout,
* e.g. if the port is already in use, reject the promise.
*/
timeoutId = setTimeout(() => {
rejectOnce(new Error('Timeout: Appium did not start within expected time'))
}, timeout)
/**
* reject promise if Appium does not start within given timeout,
* e.g. if the port is already in use
*
* @param err - error to reject with
*/
const rejectOnce = (err: Error) => {
if (!errorCaptured) {
errorCaptured = true
clearTimeout(timeoutId)
reject(err)
}
}
process.stdout.on('data', (data) => {
if (data.includes('Appium REST http interface listener started')) {
outputBuffer += data.toString()
if (outputBuffer.includes('Appium REST http interface listener started')) {
log.info(`Appium started with ID: ${process.pid}`)
clearTimeout(timeoutId)
resolve(process)
}
})
@ -171,19 +201,23 @@ export default class AppiumLauncher implements Services.ServiceInstance {
/**
* only capture first error to print it in case Appium failed to start.
*/
process.stderr.once('data', (err) => { error = err })
process.stderr.once('data', (data) => {
error = data.toString() || 'Appium exited without unknown error message'
log.error(error)
rejectOnce(new Error(error))
})
process.once('exit', exitCode => {
process.once('exit', (exitCode: number) => {
let errorMessage = `Appium exited before timeout (exit code: ${exitCode})`
if (exitCode === 2) {
errorMessage += '\n' + (error?.toString() || 'Check that you don\'t already have a running Appium service.')
} else if (error) {
errorMessage += `\n${error.toString()}`
} else if (errorCaptured) {
errorMessage += `\n${error?.toString()}`
}
if (exitCode !== 0) {
log.error(errorMessage)
}
reject(new Error(errorMessage))
rejectOnce(new Error(errorMessage))
})
})
}

View file

@ -395,7 +395,7 @@ describe('Appium launcher', () => {
vi.mocked(spawn).mockReturnValue(new MockCustomFailingProcess(2) as unknown as cp.ChildProcess)
const error = await launcher.onPrepare().catch((err) => err)
const expectedError = new Error('Appium exited before timeout (exit code: 2)\nError: Uups')
const expectedError = new Error('Error: Uups')
expect(error).toEqual(expectedError)
})
@ -536,15 +536,37 @@ describe('Appium launcher', () => {
})
describe('_startAppium', () => {
test('should propagate error messgae', async () => {
test('should propagate error message', async () => {
const origSpawn = await vi.importActual<typeof cp>('node:child_process').then((m) => m.spawn)
vi.mocked(spawn).mockImplementationOnce(origSpawn)
const launcher = new AppiumLauncher({}, [], {} as any)
await expect(launcher['_startAppium'](
'node',
['-e', '(() => { process.stderr.write(\'something went wrong\\n\'); throw new Error(\'ups\') })()']
['-e', '(() => { process.stderr.write(\'something went wrong\\n\'); throw new Error(\'ups\') })()'], 2000
)).rejects.toEqual(expect.objectContaining({ message: expect.stringContaining('something went wrong') }))
})
test('should validate timeout parameter (timeout reach)', async () => {
const origSpawn = await vi.importActual<typeof cp>('node:child_process').then((m) => m.spawn)
vi.mocked(spawn).mockImplementationOnce(origSpawn)
const launcher = new AppiumLauncher({}, [], {} as any)
await expect(launcher['_startAppium'](
'node',
['-e', '(() => { setTimeout(() => { console.log(JSON.stringify({message: \'done\'})); }, 5000); })()'],
1000
)).rejects.toEqual(expect.objectContaining({ message: expect.stringContaining('Timeout: Appium did not start within expected time') }))
})
test('should validate timeout parameter (no timeout reach)', async () => {
const origSpawn = await vi.importActual<typeof cp>('node:child_process').then((m) => m.spawn)
vi.mocked(spawn).mockImplementationOnce(origSpawn)
const launcher = new AppiumLauncher({}, [], {} as any)
await expect(launcher['_startAppium'](
'node',
['-e', '(() => { setTimeout(() => { console.log(JSON.stringify({message: \'Appium REST http interface listener started\'})); }, 3000); })()'],
10000
)).resolves.toEqual(expect.objectContaining({ spawnargs: expect.arrayContaining(['-e', expect.any(String)]) }))
})
})
afterEach(() => {