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

Dev test #177

Merged
merged 2 commits into from
Apr 9, 2024
Merged

Dev test #177

merged 2 commits into from
Apr 9, 2024

Conversation

msimerson
Copy link
Contributor

@msimerson msimerson commented Apr 4, 2024

  • test: switch runner from mocha to node builtin
  • test: remove useless done
    • they are only required for callback style async tests
  • pkg: rm .travis.yml
  • ci: update test runners
  • ci: automatically test node LTS versions
  • ci: update lint workflow

Copy link
Owner

@whitequark whitequark left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up!

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@msimerson
Copy link
Contributor Author

msimerson commented Apr 8, 2024

Several changes here:

  • dep(eslint): restored declaration of dependency on v8
  • move the test file into lib (next to the code it runs). Naming files with *.test.js and putting them next to the files they test is apparently a pattern. Your thoughts?
  • mocha -> node --test
    • ci: drop testing for older nodes that lack support for node --test
  • ci: updated lint workflow

- test: remove useless done
  - they are only required for callback style async tests
- test: move test file into lib
- pkg: rm .travis.yml
- ci: update test runners
- ci: automatically test LTS versions
- ci: update lint workflow
@msimerson
Copy link
Contributor Author

msimerson commented Apr 8, 2024

naming files with *.test.js and putting them next to the files they test is apparently a pattern.

Something I just thought of and don't like, is that putting the test file in lib publishes it on npm. That's a small deal, but with 36m weekly downloads, the smallness has a very large multiplier. I've talked myself out of it. 😲 Sorry for the noise.

Note to self: the NPM docs say that putting a .npmignore in a subdirectory will override [files], so one could echo '*.test.js' > lib/.npmignore. With dozens of npm modules that (still) use the ./test/ convention, I'm not yet convinced.

@whitequark whitequark mentioned this pull request Apr 9, 2024
@whitequark whitequark merged commit f523168 into whitequark:main Apr 9, 2024
6 checks passed
@whitequark
Copy link
Owner

Thanks!

@msimerson msimerson deleted the dev-test branch April 9, 2024 14:32
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.

2 participants