ssbc / ssb-keys

keyfile operations for ssb
36 stars 26 forks source link

create test-verbose script, make test script unverbose #77

Closed staltz closed 4 years ago

staltz commented 4 years ago

I think Dominic's style of writing tests usually involves console.logs sprinkled here and there while we debugged their outputs. That's okay for debugging tests when you're writing tests, but when you're just running the tests (that you already trust to be written correctly), you don't want to see all that input.

This PR puts a if (process.env.VERBOSE_TESTS) check before every console.log and then adds a new script test-verbose which has that env var enabled. Thus, when you run npm test, it will be "silent", but if you run npm run test-verbose it will give you all those console.log outputs. CI should use npm test.

davegomez commented 4 years ago

Is this writing style (without curly braces) planned to change in the future with the inclusion of ESLint and Prettier?

if (condition)
  doSomething();
staltz commented 4 years ago

@davegomez Prettier is already included and does not add a newline to those if conditions.

davegomez commented 4 years ago

Prettier will add the curly braces.

But it won't do it automatically, it has to be run over the project files with the --write flag in the CLI, the file being affected by the editor plugin, or the script runing over the staged or changed files during CI pipelines.

staltz commented 4 years ago

@davegomez Prettier won't do that, see this example run:

➜  ssb-keys git:(verbose-tests) $(npm bin)/prettier --write test/*.js
test/box-unbox.js 63ms
test/fs.js 22ms
test/index.js 55ms
test/secretbox.js 11ms
test/weird.js 10ms

The lines are grey in my terminal, which means Prettier didn't detect anything to be changed, and the git status doesn't show any changed lines.

PS: we added https://github.com/ssb-js/ssb-keys/pull/73 precisely so we don't have to discuss code formatting, it runs automatically upon git commit.

davegomez commented 4 years ago

Oh, I guess Prettier doesn't have any preference about that particular case then.

In your personal opinion. Do you think is a good practice?

staltz commented 4 years ago

Prettier still leaves some code formatting options to the code author. Like if you put newlines in between code snippets, Prettier allows that. If you remove the newlines, Prettier allows that too.

In this specific case, I can choose to write if (condition) action() or if (condition) { action() }. If I choose the first, Prettier will put it in one line. If I choose the second, Prettier will add newlines for the curly braces. But I can choose between curly braces or no curly braces.

Overall, my opinion is that we should never discuss code formatting. Code authors can choose differently depending on the case, Prettier will apply something, but overall it's counterproductive to get stuck on these details.

davegomez commented 4 years ago

Overall, my opinion is that we should never discuss code formatting. Code authors can choose differently depending on the case, Prettier will apply something, but overall it's counterproductive to get stuck on these details.

I agree and my idea is not to be stuck. It was more curiosity than anything else because I don't have a strong opinion about it either.

staltz commented 4 years ago

Rebased to resolve git conflicts. :)