Skip to content
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

Jump to bottom

Allow user to specify a return code when the test fails #137

Merged
merged 3 commits into from
May 6, 2020
Merged

Allow user to specify a return code when the test fails #137

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dfb05a6
feat: add option fail-status-code to run command
levanhieu8396 Apr 21, 2020
c8505a9
fix: add type for fail-status-code
levanhieu8396 Apr 21, 2020
614051a
chore: throw fail result as error
levanhieu8396 May 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/cli/src/cmd/run.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ interface RunArguments extends Arguments {
slowMo?: number
'work-root'?: string
'test-data-root'?: string
'fail-status-code'?: number
ivanvanderbyl marked this conversation as resolved.
Show resolved Hide resolved
}

function setupDelayOverrides(args: RunArguments, testSettingOverrides: TestSettings) {
Expand Down Expand Up @@ -85,6 +86,7 @@ const cmd: CommandModule = {
runEnv: initRunEnv(workRootPath, testDataPath),
testSettingOverrides: {},
persistentRunner: false,
failStatusCode: args['fail-status-code'],
}

if (args.loopCount) {
Expand Down Expand Up @@ -188,6 +190,10 @@ const cmd: CommandModule = {
.option('verbose', {
describe: 'Verbose mode',
})
.option('fail-status-code', {
describe: 'Exit code when the test fails',
type: 'number',
})
.positional('file', {
describe: 'the test script to run',
})
Expand Down
14 changes: 9 additions & 5 deletions packages/core/src/Element.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,13 @@ export interface ElementOptions {
testObserverFactory?: (t: TestObserver) => TestObserver
persistentRunner: boolean
testCommander?: TestCommander
failStatusCode?: number
}

export function runUntilExit(fn: () => Promise<void>) {
export function runUntilExit(fn: () => Promise<number>) {
fn()
.then(() => {
process.exit(0)
.then((code: number) => {
process.exit(code)
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is failStatusCode shouldn't this only get called within catch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean we throw the failStatusCode as an error to catch is here, instead of return it as a result of running tests?

Copy link
Contributor

Choose a reason for hiding this comment

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

If a test completes without error, we should always exit 0, if a test fails or throws and error or for whatever reason is not a clean exit, it should exit with failStatusCode

Copy link
Contributor

Choose a reason for hiding this comment

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

Still waiting on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the function runUntilExit, and handled exit logic in runCommandLine. Since runCommandLine receives failStatusCode and other ElementOptions, I think it's better to handle exit there

})
.catch(err => {
console.log('Element exited with error')
Expand All @@ -41,7 +42,7 @@ export function runUntilExit(fn: () => Promise<void>) {
})
}

export async function runCommandLine(opts: ElementOptions): Promise<void> {
export async function runCommandLine(opts: ElementOptions): Promise<number> {
const { logger, testScript, clientFactory } = opts

// TODO proper types for args
Expand Down Expand Up @@ -95,5 +96,8 @@ export async function runCommandLine(opts: ElementOptions): Promise<void> {
return new EvaluatedScript(opts.runEnv, await mustCompileFile(testScript, testScriptOptions))
}

await runner.run(testScriptFactory)
const result = await runner.run(testScriptFactory)
const exitCode = opts.failStatusCode ? opts.failStatusCode : 0

return result ? 0 : exitCode
}
20 changes: 12 additions & 8 deletions packages/core/src/Runner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export interface TestCommander {
}

export interface IRunner {
run(testScriptFactory: AsyncFactory<EvaluatedScript>): Promise<void>
run(testScriptFactory: AsyncFactory<EvaluatedScript>): Promise<boolean>
stop(): Promise<void>
}

Expand Down Expand Up @@ -109,12 +109,12 @@ export class Runner {
return
}

async run(testScriptFactory: AsyncFactory<EvaluatedScript>): Promise<void> {
async run(testScriptFactory: AsyncFactory<EvaluatedScript>): Promise<boolean> {
const testScript = await testScriptFactory()

this.clientPromise = this.launchClient(testScript)

await this.runTestScript(testScript, this.clientPromise)
return this.runTestScript(testScript, this.clientPromise)
}

async launchClient(testScript: EvaluatedScript): Promise<PuppeteerClient> {
Expand All @@ -137,8 +137,8 @@ export class Runner {
async runTestScript(
testScript: EvaluatedScript,
clientPromise: Promise<PuppeteerClient>,
): Promise<void> {
if (!this.running) return
): Promise<boolean> {
if (!this.running) return true

let testToCancel: Test | undefined

Expand Down Expand Up @@ -173,6 +173,7 @@ export class Runner {
await test.beforeRun()

const cancelToken = new CancellationToken()
let result = false

this.looper = new Looper(settings, this.running)
this.looper.killer = () => cancelToken.cancel()
Expand All @@ -181,7 +182,7 @@ export class Runner {

const startTime = new Date()
try {
await test.runWithCancellation(iteration, cancelToken)
result = await test.runWithCancellation(iteration, cancelToken)
} catch (err) {
this.logger.error(
`[Iteration: ${iteration}] Error in Runner Loop: ${err.name}: ${err.message}\n${err.stack}`,
Expand All @@ -193,7 +194,7 @@ export class Runner {
})

this.logger.info(`Test completed after ${this.looper.iterations} iterations`)
return
return result
} catch (err) {
if (err instanceof TestScriptError) {
this.logger.error('\n' + err.toStringNodeFormat())
Expand All @@ -210,6 +211,8 @@ export class Runner {
if (testToCancel !== undefined) {
await testToCancel.cancel()
}

return false
}
}

Expand Down Expand Up @@ -284,7 +287,7 @@ export class PersistentRunner extends Runner {
}
}

async run(testScriptFactory: AsyncFactory<EvaluatedScript>): Promise<void> {
async run(testScriptFactory: AsyncFactory<EvaluatedScript>): Promise<boolean> {
this.testScriptFactory = testScriptFactory

// TODO detect changes in testScript settings affecting the client
Expand All @@ -293,5 +296,6 @@ export class PersistentRunner extends Runner {
this.rerunTest()
await this.waitUntilStopped()
// return new Promise<void>((resolve, reject) => {})
return true
}
}
2 changes: 1 addition & 1 deletion packages/core/src/runtime/ITest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface ITest {
cancel(): Promise<void>
beforeRun(): Promise<void>
run(iteration?: number): Promise<void>
runWithCancellation(iteration: number, cancelToken: CancellationToken): Promise<void>
runWithCancellation(iteration: number, cancelToken: CancellationToken): Promise<boolean>
runStep(
testObserver: TestObserver,
browser: Browser<Step>,
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/runtime/Test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export default class Test implements ITest {
public async runWithCancellation(
iteration: number,
cancelToken: CancellationToken,
): Promise<void> {
): Promise<boolean> {
console.assert(this.client, `client is not configured in Test`)

const testObserver = new ErrorObserver(
Expand Down Expand Up @@ -154,7 +154,7 @@ export default class Test implements ITest {
cancelToken.promise,
])

if (cancelToken.isCancellationRequested) return
if (cancelToken.isCancellationRequested) return true

if (this.failed) {
console.log('failed, bailing out of steps')
Expand All @@ -171,6 +171,8 @@ export default class Test implements ITest {

// TODO report skipped steps
await testObserver.after(this)

return !this.failed
}

get currentURL(): string {
Expand Down
2 changes: 1 addition & 1 deletion packages/flood-runner/src/Grid.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { initConfig } from './initConfig'
import { startConcurrencyTicker } from './tickerInterval'
import { initInfluxReporter } from './initInfluxReporter'

export async function run(file: string): Promise<void> {
export async function run(file: string): Promise<number> {
const gridConfig = initConfig()
const influxReporter = initInfluxReporter(gridConfig)

Expand Down