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

Replace jest with mocha #509

Merged
merged 1 commit into from Nov 26, 2021
Merged

Replace jest with mocha #509

merged 1 commit into from Nov 26, 2021

Conversation

cycraig
Copy link
Contributor

@cycraig cycraig commented Nov 25, 2021

Description of change

Use mocha instead of jest to execute the Node.js Wasm binding examples as tests.

This replacement was necessary due to problems with async promises and timers when using jest, which became an issue for us after #455 since publishing now uses a timer to delay retry requests.
This only affects the tests; the examples themselves run fine individually using Node.js directly.

Also updates the package-lock.json file to version 2 incidentally, since we're using Node 16 LTS as the minimum-supported version now.

Type of change

Add an x to the boxes that are relevant to your changes.

  • Bug fix (a non-breaking change which fixes an issue)
  • Enhancement (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Fix

How the change has been tested

Node.js and browser examples pass locally.

npm run build && npm run build:examples && npm run test:node && npm run test:browser

The examples workflow also succeeds: https://github.com/iotaledger/identity.rs/actions/runs/1503177178

Change checklist

Add an x to the boxes that are relevant to your changes.

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Fixes Node tests not running due to jest trying to override timers.
@cycraig cycraig added the Bug Something isn't working. label Nov 25, 2021
@cycraig cycraig mentioned this pull request Nov 25, 2021
10 tasks
@cycraig
Copy link
Contributor Author

cycraig commented Nov 25, 2021

Not really. I suspect it's something to do with how wasm-timer works: https://github.com/PhilippGackstatter/wasm-timer
But there's no easy way to check since it's compiled to opaque Wasm code.

There are lots of posts online detailing problems with jest when using async and timers. E.g. https://newbedev.com/jest-timer-and-promise-don-t-work-well-settimeout-and-async-function
However, they all presume to be calling useFakeTimers so it could be a red-herring.

A related theory is that something in wasm-timer awaits a promise that never resolves due to it blocking a promise queue or something. Related issue: jestjs/jest#2157

@cycraig cycraig merged commit 665d66b into dev Nov 26, 2021
@cycraig cycraig deleted the fix/node-tests branch November 26, 2021 09:31
@cycraig cycraig added No changelog Excludes PR from becoming part of any changelog Wasm Related to Wasm bindings. Becomes part of the Wasm changelog labels Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working. No changelog Excludes PR from becoming part of any changelog Wasm Related to Wasm bindings. Becomes part of the Wasm changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants