Skip to content

Conversation

@robinborst95
Copy link
Contributor

@robinborst95 robinborst95 commented Apr 4, 2022

After the pkg-dir upgrade in #427 the bin/cli command failed, but this was not caught because there were no tests for it, so I added that with this PR.

About the tests themselves: I'm not 100% sure if this is the way to go, but these are at least simple tests that showed that the command was broken before the downgrade fixed it. I'm happy to get any feedback on how to improve this.

@@ -0,0 +1,3 @@
{
"name": "missing-translations"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When running the bin/cli script from a fixture, the let rootDir = pkgDir.sync(); line would return the root of this whole repo, because there is no package.json in fixtures. Adding it with just a name fixed this.

"eslint-config-prettier": "8.5.0",
"eslint-plugin-node": "11.1.0",
"eslint-plugin-prettier": "4.0.0",
"execa": "^5.0.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dependency actually came with Jest already, hence no changes in yarn.lock. Also note that version 6.x doesn't work here, because of the same reason that requireing it doesn't work then.

afterEach(async function () {
await execa('git', ['checkout', 'HEAD', 'fixtures/remove-unused-translations/translations'], {
cwd: __dirname,
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The auto-fix option actually changes the files, so this makes sure those changes are undone. It does feel a bit specific though, so any other ideas are welcome :)

@robinborst95 robinborst95 marked this pull request as ready for review April 4, 2022 11:47
@robinborst95 robinborst95 changed the title Fix running from the command line broken after pkg-dir upgrade Add tests for running from command line Apr 16, 2022
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

Successfully merging this pull request may close these issues.

1 participant