feat: add root option to afterReset hooks of type command

[jcgsville]

Description

Fix for issue 132. afterReset actions of type command now respect the root property in the action spec

Performance impact

I can't imagine a performance cost to this

Security impact

None that I'm aware of

Checklist

• My code matches the project's code style and yarn lint:fix passes.
• I've added tests for the new feature, and yarn test passes.
• I have detailed the new feature in the relevant documentation.
• I have added this feature to 'Pending' in the RELEASE_NOTES.md file (if one exists).
• If this is a breaking change I've explained why.

I wasn't able to get my unit tests to run locally. Happy to try to write a test for this if you have any advice on what I'm missing? This was the behavior when I checked out the commit before mine so none of my changes are involved here.

yarn test
yarn run v1.22.10
$ yarn lint && depcheck --ignores @types/jest,eslint_d,tslib && yarn run test:only --ci
$ yarn prettier:check && eslint --ext .js,.jsx,.ts,.tsx,.graphql .
$ prettier --ignore-path .eslintignore --check '**/*.{js,jsx,ts,tsx,graphql,md,json}'
Checking formatting...
docs/docker/README.md
docs/idempotent-examples.md
README.md
Code style issues found in the above file(s). Forgot to run Prettier?
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Also, I don't see a RELEASE_NOTES.md, so I made no additions there.

[benjie]

Huh! Looks like we weren't running prettier in CI?! Fixed in #138 - please rebase, then hopefully you can run the tests :)

[benjie]

I'm worried about unintended consequences of this, so we should probably be clear in the documentation exactly what this is expected to be used for.

[jcgsville]

@benjie I think this PR is ready to go with the exception of the release notes checkbox. Not sure if I'm missing where to add those?

What's changed since your 👀 were on this:

1. I added more detail to the readme to make clear the behavior of the root property for command actions.
2. I modified GM_DBUSER to be undefined when root = true at your suggestion to avoid confusion. I also noted this in the readme.
3. I added a unit test testing that afterReset command actions with root: true behave as intended
4. I added a bonus unit test testing that afterReset SQL actions with root: true behave as intended

Apologies that I didn't realize force pushing to my branch would break Github's requested changes. The change you requested was a small tweak to the README which I implemented.

Finally, as I was working on the unit tests, I found a separate bug in makeRootDatabaseConnectionString(). This should probably be a separate PR to fix this. If it makes more since for it to be one, let me know.

The 🐛 : when setting rootConnectionString to just the db name, the connection string used when root: true incorrectly had a slash prepended to the database name.

For example, when rootConnectionString: "postgres", connectionString: "postgres://localhost/app_db" the connection string when root: true should be postgres but it was /postgres.

I noticed this because the unit tests default rootConnection string was just postgres and the assertions were failing unexpectedly. I've modified settings.ts:420 to fix it, and I believe this change produces the desired behavior. But it's worth your 👀 to make sure I didn't misunderstand something. I also added a unit test to cover this case. I believe the existing unit tests on this function were sufficient to prove that this change introduced no regressions.

[jcgsville]

@benjie know of anything left before this can be merged?

[benjie]

Great work, some small tweaks needed 👍

[jcgsville]

I addressed the recommendations to fix the tests. Last thing pending is resolution surrounding the pathname vs dbname debacle

[benjie]

Looks good to me! 👍

[benjie]

My new test didn't pass linting :(

[benjie]

I tried fixing it using prettier playground but that wasn't enough; please reformat 👍

[jcgsville]

Thanks for your thorough reviews!

[benjie]

Of course it was the trailing comma 🤦‍♂️

[benjie]

Released in 1.3.0 🙌