Add --quiet-exit-code and make it default --rich-events behavior #357
Conversation
Fixes elastic#356 Accomplishes the fix by making --rich-events always exit with a code of 0, even if tests fail
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪 |
There was a problem hiding this comment.
The code looks good, But I would like to propose another solution. Instead of adding a flag specific for HB, we could exit with non zero code for CI only environments?
https://www.npmjs.com/package/is-ci, we can use this. Thoughts?
|
I'd rather avoid the dependency, and while I did say I should mention, the flag is only there for completeness, since it's turned on by default with |
__tests__/cli.test.ts
Outdated
| @@ -41,6 +41,17 @@ describe('CLI', () => { | |||
| expect(await cli.exitCode).toBe(1); | |||
| }); | |||
|
|
|||
| describe('with --quiet-exit-code', () => { | |||
| it('runs the suites, exiting with 0', async () => { | |||
There was a problem hiding this comment.
nit: would move it out of describe block to keep in same level as other test.
| @@ -77,6 +88,7 @@ describe('CLI', () => { | |||
| it('mimick new heartbeat with `--rich-events` flag', async () => { | |||
| const cli = new CLIMock([ | |||
| join(FIXTURES_DIR, 'fake.journey.ts'), | |||
| join(FIXTURES_DIR, 'error.journey.ts'), | |||
There was a problem hiding this comment.
We need to increase the timeout for this specific test.
it ('asda', callback, timeout: number), Increasing with 10 secs should do it as we now test for 2 files and also screenshots which takes more than 5 seconds.
__tests__/cli.test.ts
Outdated
| @@ -94,7 +104,7 @@ describe('CLI', () => { | |||
| }); | |||
|
|
|||
| expect(await cli.exitCode).toBe(0); | |||
| }); | |||
| }, 30); | |||
There was a problem hiding this comment.
nit: this is in milliseconds.
Fixes #356
Accomplishes the fix by making --rich-events always exit with a code of 0, even if tests fail