Skip to content

Conversation

@sorinh
Copy link

@sorinh sorinh commented Jul 21, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change

The readfile benchmark of fs type was throwing an exception because the benchmark tried to read the file after it got unlinked at the end of the run.

Signed-off-by: Sorin Baltateanu sorin.baltateanu@intel.com

Signed-off-by: Sorin Baltateanu <sorin.baltateanu@intel.com>
@nodejs-github-bot nodejs-github-bot added the benchmark Issues and PRs related to the benchmark subsystem. label Jul 21, 2016
@mscdex mscdex added the fs Issues and PRs related to the fs subsystem / file system. label Jul 21, 2016
@jasnell
Copy link
Member

jasnell commented Aug 8, 2016

I'm unable to recreate the exception on macos and I'm not sure if introducing a timer here is the best approach. Pinging @mscdex for his thoughts...

@AndreasMadsen
Copy link
Member

I remember having seen that error too, but it should be fixed after 8bb59fd landed. Though this fix may be better.

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

This stil seems to make sense to me.

CI: https://ci.nodejs.org/job/node-test-commit/8078/

jasnell pushed a commit that referenced this pull request Mar 24, 2017
PR-URL: #7818
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Mar 24, 2017

Landed in d9b0e4c

@jasnell jasnell closed this Mar 24, 2017
@mscdex
Copy link
Contributor

mscdex commented Mar 25, 2017

I'm confused as to why this was merged when there was already a process.exit(0); added much earlier?

MylesBorins pushed a commit that referenced this pull request Mar 28, 2017
PR-URL: #7818
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Mar 28, 2017
@italoacasas italoacasas mentioned this pull request Apr 10, 2017
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmark Issues and PRs related to the benchmark subsystem. fs Issues and PRs related to the fs subsystem / file system.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants