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

chore(tests): migrate to node test runner #717

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

minht11
Copy link
Contributor

@minht11 minht11 commented Jul 11, 2024

Remove chai/mocha in favor of native node test runner and asserts.
Use tsx to load ts test files.

All tests but one pass.

@Borewit Borewit self-assigned this Jul 12, 2024
@Borewit Borewit added the DevOps DevOps / Continuous integration label Jul 12, 2024
@Borewit
Copy link
Owner

Borewit commented Jul 14, 2024

Interesting changes @minht11

I still have a few challenges, and they may be not be directly related to these change.
IntellijJ does not understand the test results, I think this is the combination with TypeScript.
So I still have difficulties with easy debugging of individual unit tests.

Copy link
Owner

@Borewit Borewit left a comment

Choose a reason for hiding this comment

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

You even migrated some decencies from Buffer to UInt8Array

package.json Outdated Show resolved Hide resolved
@Borewit
Copy link
Owner

Borewit commented Jul 14, 2024

Do you mind to rebase this PR @minht11 ?

@minht11 minht11 force-pushed the chore/migrate-to-node-test-runner branch 3 times, most recently from b484469 to 00bd26e Compare July 14, 2024 17:44
@minht11
Copy link
Contributor Author

minht11 commented Jul 14, 2024

Rebased it.
Concerning individual tests. At least for VSCode there is node runner extension and it needs --import=tsx flag to run typescript files correctly.
.vscode/settings.json

{
    "nodejs-testing.extensions": [
      {
        "extensions": ["mts", "cts", "ts"],
        "parameters": ["--import", "tsx"]
      }
    ]
  }

You should look into if there some way to pass arguments inside IntellijJ as well.

@Borewit
Copy link
Owner

Borewit commented Jul 17, 2024

I am sorry @minht11 , I need to ask you to rebase again. You have made good changes to the unit tests, which already applied on the default branch, so I hope it is not so painful this time. I

can also rebase if you prefer.

@minht11 minht11 force-pushed the chore/migrate-to-node-test-runner branch from 00bd26e to 1fedce0 Compare July 17, 2024 14:55
@minht11
Copy link
Contributor Author

minht11 commented Jul 17, 2024

No problem. Rebased it.

@Borewit
Copy link
Owner

Borewit commented Jul 17, 2024

You can exclude Node.js version 16 from the Matrix, to pass all the unit tests:

node-version: [16.x, 18.x, 20.x, 22.x]

@minht11 minht11 force-pushed the chore/migrate-to-node-test-runner branch from 1fedce0 to d8eace3 Compare July 17, 2024 15:09
@Borewit
Copy link
Owner

Borewit commented Aug 5, 2024

Sorry, @minht11, can you please rebase another time?

@minht11 minht11 force-pushed the chore/migrate-to-node-test-runner branch from d8eace3 to 056449d Compare August 5, 2024 19:01
@minht11 minht11 force-pushed the chore/migrate-to-node-test-runner branch from 0398343 to 93b009a Compare August 5, 2024 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DevOps DevOps / Continuous integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants