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

Add tests #5

Merged
merged 6 commits into from
Jan 28, 2018
Merged

Add tests #5

merged 6 commits into from
Jan 28, 2018

Conversation

lukejacksonn
Copy link
Owner

Handles no children return with empty array instead of empty string, adds tests with tead. Any tips or ideas for tests?

package.json Outdated
"license": "MIT"
"license": "MIT",
"dependencies": {
"tead": "^0.4.3"
Copy link

Choose a reason for hiding this comment

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

Isn't tead a devDependency?

Copy link

Choose a reason for hiding this comment

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

Agree.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ooops. Thanks!

package.json Outdated
@@ -4,7 +4,13 @@
"description": "Transforms arrays into virtual dom trees",
"main": "index.js",
"module": "index.js",
"scripts": {
"test": "tead"
Copy link

Choose a reason for hiding this comment

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

Why not adding code coverage as well :tead --coverage ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Will do!

Copy link
Owner Author

Choose a reason for hiding this comment

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

Wait.. that wasn't as simple as it sounded 😅

$ tead --coverage
/bin/sh: npx: command not found
Done in 0.22s.

Copy link

Choose a reason for hiding this comment

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

Are you using Node.js < 8?

See the note here: https://github.com/teadjs/tead#npx

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm on v8.1.3 but I am not using npx anywhere in the project, or at all in fact

Copy link

Choose a reason for hiding this comment

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

Sorry, forgot thatnpx wasn't introduced in Node.js 8.0.0, but comes bundled with npm >= 5.2.0, which comes bundled with Node.js >= 8.2.0. 😂

It's not like I've already posted about this and forgot or anything 🙄

npx is a pretty cool tool that allows you to run a command from any package on npm without having to add it as a dependency, but the user still has control over which version they run. First it checks if the package is installed in node_modules, then globally, and finally it just downloads a copy of the latest version on-demand from npm as a last resort. 😎

Copy link

Choose a reason for hiding this comment

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

FYI:
screen shot 2018-01-27 at 13 32 04
screen shot 2018-01-27 at 13 32 17

@lukejacksonn
Copy link
Owner Author

Going to merge this now. Much improvement on no tests 😅 thanks @Mytrill and @okwolf

@lukejacksonn lukejacksonn deleted the add-tests branch January 28, 2018 13:22
@okwolf
Copy link

okwolf commented Jan 28, 2018

@lukejacksonn depending on how much more development/refactoring you're going to do on the project, you might also want a script that calls tead --watch like npm run watch or npm run test:watch.

@lukejacksonn
Copy link
Owner Author

@okwolf ahh yeh.. 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

Successfully merging this pull request may close these issues.

3 participants