Skip to content
This repository was archived by the owner on Apr 11, 2024. It is now read-only.

allow trigger error, for better async test#205

Open
bluelovers wants to merge 3 commits into
bestiejs:mainfrom
bluelovers:master
Open

allow trigger error, for better async test#205
bluelovers wants to merge 3 commits into
bestiejs:mainfrom
bluelovers:master

Conversation

@bluelovers

Copy link
Copy Markdown

No description provided.

@bluelovers

Copy link
Copy Markdown
Author

* 'master' of https://github.com/bestiejs/benchmark.js:
  Remove sites from copyright header and package.json.
  Remove npm ci use from travis since it is called automatically.
  Update deps.
  chore: upgrade npm (bestiejs#202)
  chore: use npm ci (bestiejs#201)
  chore: add Node.js 8 and 10 (bestiejs#199)
  chore: remove Node.js 4 (bestiejs#203)

@prantlf prantlf left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You should drop the changes to package.json, if you want to get this request merged.

Comment thread benchmark.js
bench = clone._original;

var event = Event('error');
clone.error = event.message = error;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is it not weird to set the error object as both error and message? Why not just the event.error? Or you could set error.message to event.message.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yes I agree.

@Uzlopak

Uzlopak commented Mar 9, 2022

Copy link
Copy Markdown

I like this change. I will adapt this to my fork.

With this I would add a isAsyncFunction check like the one I implemented in mongoose. Then if you pass an async function to the benchmarks, it should wrap the async function in the existing defer mode and resolve and reject can be assigned and it should work like a charm.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Development

Successfully merging this pull request may close these issues.

4 participants