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.

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

Pass through non-zero exit codes #2175

Closed
tomholub opened this issue Jul 4, 2019 · 6 comments
Closed

Pass through non-zero exit codes #2175

tomholub opened this issue Jul 4, 2019 · 6 comments

Comments

@tomholub
Copy link

tomholub commented Jul 4, 2019

We have a shell sript that retries tests based on exit codes. For example, 0 is pass, 1 is fail, 2 is retriable fail.

When a retriable fail is detected during an ava test, I will hard-exit the process with appropriate exit code. Typically during setup:

ava.before('set up important things before test, some of which may fail badly', async t => {
  // ..
  if(failedBadly) {
    process.exit(2);
  }
});

What happens:

✖ build/test/test/source/test.js exited with a non-zero exit code: 2

0 tests passed
1 uncaught exception

npm ERR! code ELIFECYCLE
npm ERR! errno 1

It looks like ava will swallow the errcode and return 1 instead, although ava does seem to know that it was exit code 2.

It would be good to pass the errcode along.

Tested on ubuntu, with ava 1.0.0-beta.4 and --verbose.

@novemberborn
Copy link
Member

I can see this could be useful, but I'm not sure I'm comfortable linking the outcome of running AVA to how the worker process itself behaved. Might there be other ways of tackling this problem? Why can't you retry in the before hook itself?

@novemberborn
Copy link
Member

Tested on ubuntu, with ava 1.0.0-beta.4 and --verbose.

You really should upgrade to the latest AVA version by the way 😉

@tomholub
Copy link
Author

tomholub commented Jul 7, 2019

I just wanted to make sure to do a 100% clean retry, with no state left over from the last time we tried to set up and run the tests, and maybe with lower concurrency.

We did end up working around this differently, so it's up to you if you want to proceed with this or pass. It would come handy.

You really should upgrade to the latest AVA version by the way wink

Will do :)

@novemberborn
Copy link
Member

We did end up working around this differently

Would you be able to share your workaround?

@tomholub
Copy link
Author

tomholub commented Jul 7, 2019

Retry tests a few times before giving up. Very roughly (I hastily edited the code to remove timeouts, puppeteer browser pool, other stuff):

Define a test:

ava.test('some test', retriableTest(async t => {
  await doFlakyStuff();
}));

Using this test definition helper:

const retriableTest = (cb: (t: AvaContext, browser: BrowserHandle) => Promise<void>): ava.Implementation<{}> => {
  return async (t: AvaContext) => {
    await doRetryTest(cb, t);
    t.pass();
  };
};

Which in turn uses the retry logic:

const doRetryTest =  async (cb: (t: AvaContext) => void, t: AvaContext) => {
  t.totalAttempts = 3; // actually loaded from config
  for (let attemptNumber = 1; attemptNumber <= t.totalAttempts; attemptNumber++) {
    t.attemptNumber = attemptNumber;
    t.attemptText = `(attempt ${t.attemptNumber} of ${t.totalAttempts})`;
    try {
      await cb(t);
      return;
    } catch (err) {
      if (t.attemptNumber! < t.totalAttempts!) {
        t.log(`${t.attemptText} Retrying: ${String(err)}`);
      } else {
        t.log(`${t.attemptText} Failed:   ${err instanceof Error ? err.stack : String(err)}`);
        t.fail(`[ALL RETRIES FAILED for ${t.title}]`);
      }
    }
  }
}

The tests then look like this:

image

@novemberborn
Copy link
Member

@tomholub I think you'll like t.try() when we land #1947 😄

I'm closing this for now, since you've managed to work around the issue in line with enhancements we're already trying to land.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants