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

refactor(test): replaces mocha with node:test #827

Closed
wants to merge 7 commits into from

Conversation

sverweij
Copy link
Owner

@sverweij sverweij commented Aug 5, 2023

Description

  • replaces mocha with node:test

This won't be merged

Everything works. Had to write a little test reporter to get output that is to the point (no thousands of tests over the screen, but just a little bit of progress information and only full information if a test fails).

However, this PR is not going to be merged. I will continue to work with node:test projects that have (far) less automated tests as having no external dependencies for tests is still pretty neat. For dependency-cruiser, at this time, the drawbacks are heavier than the benefits.

Why this is not going to be merged 1/2: node:test is too slow

There's a big drawback on using node:test on this amount of tests, though - node:test is very CPU hungry and twice as slow as exactly the same tests run with mocha. This is not a problem when there's a moderate number of tests, but with ~1400 of them it really adds up.

The unix time utility emits numbers like this for node:test on my local machine:

yarn test  87,57s user 10,06s system 357% cpu 27,294 total

With mocha the same run looks like this:

yarn test  16,75s user 1,41s system 127% cpu 14,217 total

On the ci the slowdowns are comparable - see the ci runs associated with this PR.

Why this is not going to be merged 2/2: challenges in filtering

One of the things a test runner should be able to do is quickly isolate one test, or a group of tests either by name (/ regex) or by filename. In addition this should be pretty fast

  • node --test has a --test-name-pattern command line switch that does this, however
  • it filters for the test name only, not for the test name + the suite. This means it's not easy to run just the tests in a suite. Worse, if your test names within a suite are somewhat generic (it('returns a truthy value by default') and appear in multiple places
  • it's still quite slow even when targeting exactly one test - it looks like even skipping over tests takes it a while.

Motivation and Context

reduces dependencies

How Has This Been Tested?

  • green ci

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Documentation only change
  • Refactor (non-breaking change which fixes an issue without changing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • 📖

    • My change doesn't require a documentation update, or ...
    • it does and I have updated it
  • ⚖️

    • The contribution will be subject to The MIT license, and I'm OK with that.
    • The contribution is my own original work.
    • I am ok with the stuff in CONTRIBUTING.md.

@codeclimate
Copy link

codeclimate bot commented Aug 6, 2023

Code Climate has analyzed commit 9464eed and detected 0 issues on this pull request.

View more on Code Climate.

@github-actions github-actions bot added the stale label Aug 17, 2023
@jaads
Copy link

jaads commented Aug 17, 2023

nice try! 👍 too bad that it didn't worked out. it seems from a certain project size on, third party libraries are still very much needed.

@github-actions github-actions bot removed the stale label Aug 19, 2023
@github-actions github-actions bot added the stale label Aug 29, 2023
@github-actions github-actions bot closed this Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants