playwright icon indicating copy to clipboard operation
playwright copied to clipboard

[Feature] Define different exit codes to make CI/CD scripting easier

Open lightswitch05 opened this issue 3 years ago • 5 comments

It looks like process.exit(1); is used universal throughout the project when something does not go as expected. process.exit(1); is also used when everything does execute properly, but not all test cases passed. Finally, it seems there are a couple cases were exit code 0 is associated with an error.

It would be very handy for CI/CD scripting to easily determine Playwright's exit reason. For very basic support, it could be:

  • 0: Everything executed properly with all tests passing
  • 1: Everything executed properly, but with failing tests
  • 2: Something unexpected happened during execution

For my needs, these three different exit code would be good enough. I would like to run tests in the pipeline - and I have some tests that will periodically fail due to external reasons. As long as all tests are running, I would like the pipeline to be successful - or perhaps marked as unstable. On the other hand, if something new was introduced that is causing the tests to break - then that is a problem to halt the pipeline over.

Depending on how granular you want to go, you could add even more exit codes. This is what pytest does for their exit codes:

  • 0: All tests were collected and passed successfully
  • 1: Tests were collected and run but some of the tests failed
  • 2: Test execution was interrupted by the user
  • 3: Internal error happened while executing tests
  • 4: pytest command line usage error
  • 5: No tests were collected

I'm not opposed to trying to add support for more consistent exit codes myself, but I want to make sure everyone is aligned on expected behavior before any time is spent on it.

Thanks!

lightswitch05 avatar May 11 '22 19:05 lightswitch05

@lightswitch05 Let me add some additions:

You provide example with exit codes which using in pytest. In Nodejs this exit codes are in used by node.js itself. I think each implementation should use own exit codes for not breaking environment consistancy.

As for me - this request related playwright test, since in other case, playwright is just an object with methods and properties like any other library

vitalics avatar May 11 '22 21:05 vitalics

@vitalics it makes sense to choose exit codes that do not conflict with Node's own exit codes.

I'm not advocating recreating PyTest's exit codes, but I wanted to provide an example of how exit codes could be more granular especially in context of a testing tool.

I appreciate that Playwright is a collection of objects and methods. I think this enhancement request is focused on the CLI aspect of Playwright where the CLI is the interface to Playwright and the exit code is a system-supported return type.

lightswitch05 avatar May 11 '22 22:05 lightswitch05

@lightswitch05 I understand your point. Not sure about possibilities for CLI exit codes, since you can only understand - if your file executes with/without errors. If you run the simple file with a playwright - you should take care of your own exit codes like in the example below

async function main(){
  try {
    // run playwight
  }  catch (err) {
    if(err === 'external reason') {
      process.exit(<my exit code>);
    }
  }
}

main();

I suppose each testing implementation should provide its exit codes. if we talking about python - PyTest should return their exit codes.

I would like to ask @mxschmitt to transform your request for the @playwright/test package if it's a relevant feature request - since @playwright/test is the official test runner for playwright.

Addition: jest(@playwright/test based on jest) supports only exit code 1 if at least one test has been failed by expectation failing. I do research on exit codes in nodejs and the best way - do not to break backward compatibility. But in the case of a new implementation test runner - I suggest to use exit codes > 14. E.g.

  • 0 - all tests passed
  • 1 - all tests failed
  • 14 - some tests failed
  • 15 - cancel by user
  • 7 - internal error happened while executing tests
  • 16 - command line error? (or just exit code 1)
  • 17 - No tests have been executed

vitalics avatar May 13 '22 08:05 vitalics

Anyone on the playwright team want to chime in here? It looks like the exit codes currently being used are:

  • 0: Success, or in some cases invalid arguments
  • 1: tests failed, or execution error
  • 130: SIGINT/interrupted

I think for an initial implementation, these 3 exit codes could be split up into 5 different exit codes:

  • 0 - Success
  • 1 - Tests failed
  • Invalid arguments
  • Execution error
  • SIGINT/interrupted

I'm curious what exit codes would be recommended? @vitalics brought up a interesting point about Node's own exit codes. Then there is another question about backwards compatibility, I'm not sure if the 1/130 exit codes are documented anywhere? Perhaps keep 1 and 130 and just start the others at 20 to avoid node's exit codes? After this initial implementation, if there is interest in more granularity in the exit codes, then it could just continue counting up from 20.

  • 0 - Success
  • 1 - Tests failed
  • 20 - Invalid arguments
  • 21 - Execution error
  • 130 - SIGINT/interrupted

Thoughts?

lightswitch05 avatar Jul 20 '22 02:07 lightswitch05

My issue #16309 was merged into this one. I would like Playwright to exit with status code 124 when the global timeout is hit.

Example use-case: We use the retry action that re-runs the tests once to eliminate flakiness. I do not want to retry if the first attempt timed out, but I can't know if it finished due to error or timeout from the exit status code.

Luc45 avatar Aug 06 '22 19:08 Luc45

Another good exit status code to have is: Interrupted due to max-failures reached

Luc45 avatar Aug 10 '22 16:08 Luc45

I'm in a similar spot where I'd like to pass my Azure Devops pipeline task even if a test fails. Does anyone have a workaround for this? Maybe using globalTeardown to catch the exit error?

joeflan avatar Sep 21 '22 19:09 joeflan

@joeflan for your purpose I would like to recommend wrap using script and capture error using bash/cmd.

Example

npm run test || exit 0

vitalics avatar Sep 22 '22 07:09 vitalics

@vitalics the problem with this is that the build will be marked successful even when Playwright fails because of some other issue which could prevent the tests from even running. I'm facing a similar issue using Jenkins.

If we had either

  • an option to return exit code 0 even if tests failed, or
  • well-defined exit codes that indicate whether tests failed or the execution itself failed (e.g. because of configuration errors)

then this would enable a more robust CI setup.

AndreasHae avatar Jun 23 '23 14:06 AndreasHae

I don't know if the situation with exit codes changed in the meantime, but some time ago, a new workaround became possible: The custom reporter API's onEnd callback allows you to override the status (for example replacing a 'failed' by 'passed').

https://playwright.dev/docs/api/class-reporter#reporter-on-end

Reporter is allowed to override the status and hence affect the exit code of the test runner.

eveltman-twinfield avatar Nov 04 '23 16:11 eveltman-twinfield

https://playwright.dev/docs/api/class-reporter#reporter-on-end

Hi, did you tried overriding the result using onEnd(). It wasn't changing for me when I triedm hence my query. Needed to overcome the trouble as mentioned in the subject of this issue which led to using the workaround mentioned as one of the comment (using multiple commands as part of npm script run)

irahul avatar Nov 20 '23 16:11 irahul