Skip to content

Run tests in CI for Node 18, 20, 22#966

Merged
taras merged 11 commits intov4from
tm/test-more-node-versions
Jan 22, 2025
Merged

Run tests in CI for Node 18, 20, 22#966
taras merged 11 commits intov4from
tm/test-more-node-versions

Conversation

@taras
Copy link
Copy Markdown
Member

@taras taras commented Jan 22, 2025

Motivation

I found a bug when running Effection v4 in Node 18 that was not there in Node 20. I tracked it to toReversed. I wanted to fix it but I found that we don't actually run tests in Node.

Approach

  1. Run tests in Node in CI for 18, 20, and 22.
  2. I had to replace useCommand with tinyexec because Deno.Command doesn't have a shim for Node.

@taras taras requested a review from cowboyd January 22, 2025 03:38
@taras taras changed the title Run all node tests in CI for Node 18, 20, 22 Run tests in CI for Node 18, 20, 22 Jan 22, 2025
Comment thread tasks/built-test.ts Outdated
Copy link
Copy Markdown
Member

@cowboyd cowboyd left a comment

Choose a reason for hiding this comment

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

This is a major enhancement in our coverage, awesome!

Since we now are running the full test suite including maint.test.ts in NodeJS, I think we can get rid of the test:node task in deno.json and the corresponding .mjs file which just ensured that the main() function worked, but nothing else.

@taras
Copy link
Copy Markdown
Member Author

taras commented Jan 22, 2025

@cowboyd Repurposed node:test but I removed the node.mjs file.

@taras taras merged commit e0dc265 into v4 Jan 22, 2025
@taras taras deleted the tm/test-more-node-versions branch January 22, 2025 15:05
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