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

process.exit(0) in v2.2.0 tasks limit programatic usage #118

Open
gsears opened this issue Feb 19, 2022 · 0 comments
Open

process.exit(0) in v2.2.0 tasks limit programatic usage #118

gsears opened this issue Feb 19, 2022 · 0 comments

Comments

@gsears
Copy link

gsears commented Feb 19, 2022

Hi all,

While probably well-intentioned, introducing process.exit(0) in #113 in the various tasks has limited the ability to call the code programatically. In our case, we do a lot of integration testing on the db with our main schema applied to it. As we have a considerable amount of migrations, it is not viable to rerun all migrations from scratch, so we pull a docker container with our latest version-control schema and apply local migrations to this during development (considerably speeding up the workflow).

To achieve this we have the following jest setup script:

import dotenv from 'dotenv';
import { runTaskByName } from '@fauna-labs/fauna-schema-migrate/dist/tasks';
import { createClient } from './libs/fauna';

dotenv.config();

module.exports = async () => {
  const client = createClient(process.env.FAUNADB_SECRET)();
  // @ts-expect-error 2339 Make client available for teardown
  global.client = client;
  try {
    await runTaskByName('apply', 'all', []);
    console.log('Applied migrations');
  } catch (err) {
    console.error(err);
    process.exit(1);
  }
};

This was working fine in the previous version, but the introduction of the following code kills our test setup when no migration changes are needed (therefore all the test suites that follow!)

// tasks/apply.ts
   } else {
      printMessage(`     ✅ Done, no migrations to apply`, 'success')
      process.exit(0) // Problematic
    }

I guess a solution would be to do a check for migration differences using retrieveMigrationInfo or something similar to conditionally trigger the apply task, though this would essentially be doing duplicate work already done in the apply function.

Obviously this is a niche usecase, and I definitely don't want to impede the healthy development and refactoring of the tool, but I wonder if this was 'sugar' or if it is due to other changes. I've only had a cursory look at the source code, so might have missed something!

What was the rationale for the change? Would these process.exit() calls be able to be reverted?

Thanks!

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

No branches or pull requests

1 participant