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

Test fixes for V8 6.7.165-node.0 update #47

Closed
wants to merge 2 commits into from

Conversation

cjihrig
Copy link

@cjihrig cjihrig commented Mar 27, 2018

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

Fixes: #44

v8.getHeapSpaceStatistics() now includes read_only_space
in its results. Update test-v8-stats.js to account for this.

Fixes: nodejs#44
This commit updates the following postmortem metadata constants:

- v8dbg_class_SharedFunctionInfo__code__Code
  - This is now combined with SharedFunctionInfo's function_data.
  - Renamed: v8dbg_class_SharedFunctionInfo__function_data__Object
  - V8 commit: v8/v8@51ded9d

- v8dbg_class_SharedFunctionInfo__raw_name__Object and
- v8dbg_class_SharedFunctionInfo__scope_info__ScopeInfo
  - These are now combined as name_or_scope_info.
  - Renamed: v8dbg_class_SharedFunctionInfo__name_or_scope_info__Object
  - V8 commit: v8/v8@74a663b

Fixes: nodejs#44
@@ -25,7 +25,8 @@ const expectedHeapSpaces = [
'old_space',
'code_space',
'map_space',
'large_object_space'
'large_object_space',
'read_only_space'
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: also assert the type below?

Copy link
Author

Choose a reason for hiding this comment

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

That already happens, right?

Copy link
Member

Choose a reason for hiding this comment

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

I mean add assert.strictEqual(typeof heapSpace.read_only_space, 'number'); after line 39

Copy link
Author

Choose a reason for hiding this comment

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

Oh, that's not what the test is doing. The output looks like this:

> v8.getHeapSpaceStatistics()
[ { space_name: 'read_only_space',
    space_size: 0,
    space_used_size: 0,
    space_available_size: 0,
    physical_space_size: 0 },
  { space_name: 'new_space',
    space_size: 4194304,
    space_used_size: 1962224,
    space_available_size: 100112,
    physical_space_size: 3049432 },
  { space_name: 'old_space',
    space_size: 3428352,
    space_used_size: 3343840,
    space_available_size: 552,
    physical_space_size: 3404768 },
  { space_name: 'code_space',
    space_size: 1048576,
    space_used_size: 612928,
    space_available_size: 0,
    physical_space_size: 645696 },
  { space_name: 'map_space',
    space_size: 1073152,
    space_used_size: 641344,
    space_available_size: 80,
    physical_space_size: 667456 },
  { space_name: 'large_object_space',
    space_size: 0,
    space_used_size: 0,
    space_available_size: 1516973568,
    physical_space_size: 0 } ]

So, a new object has been added with space_name equal to 'read_only_space'.

Copy link
Member

Choose a reason for hiding this comment

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

🤦‍♂️ sorry. Thanks for the explanation

@targos
Copy link
Member

targos commented Mar 27, 2018

@targos
Copy link
Member

targos commented Mar 28, 2018

Thanks. Pushed to canary-base in nodejs/node@8512818 and nodejs/node@1ba1209

@targos targos closed this Mar 28, 2018
nodejs-ci pushed a commit that referenced this pull request Aug 24, 2019
BUGFIXES

* [`27cccfbda`](npm/cli@27cccfb)
  [#223](npm/cli#223) vulns → vulnerabilities in
  npm audit output ([@sapegin](https://github.com/sapegin))
* [`d5e865eb7`](npm/cli@d5e865e)
  [#222](npm/cli#222)
  [#226](npm/cli#226) install, doctor: don't crash
  if registry unset ([@dmitrydvorkin](https://github.com/dmitrydvorkin),
  [@isaacs](https://github.com/isaacs))
* [`5b3890226`](npm/cli@5b38902)
  [#227](npm/cli#227)
  [npm.community#9167](https://npm.community/t/npm-err-cb-never-called-permission-denied/9167/5)
  Handle unhandledRejections, tell user what to do when encountering an
  `EACCES` error in the cache.  ([@isaacs](https://github.com/isaacs))

DEPENDENCIES

* [`77516df6e`](npm/cli@77516df)
  `licensee@7.0.3` ([@isaacs](https://github.com/isaacs))
* [`ceb993590`](npm/cli@ceb9935)
  `query-string@6.8.2` ([@isaacs](https://github.com/isaacs))
* [`4050b9189`](npm/cli@4050b91)
  `hosted-git-info@2.8.2`
    * [#46](npm/hosted-git-info#46)
      [#43](npm/hosted-git-info#43)
      [#47](npm/hosted-git-info#47)
      [#44](npm/hosted-git-info#44) Add support for
      GitLab subgroups ([@mterrel](https://github.com/mterrel),
      [@isaacs](https://github.com/isaacs),
      [@ybiquitous](https://github.com/ybiquitous))
    * [`3b1d629`](npm/hosted-git-info@3b1d629)
      [#48](npm/hosted-git-info#48) fix http
      protocol using sshurl by default
      ([@fengmk2](https://github.com/fengmk2))
    * [`5d4a8d7`](npm/hosted-git-info@5d4a8d7)
      ignore noCommittish on tarball url generation
      ([@isaacs](https://github.com/isaacs))
    * [`1692435`](npm/hosted-git-info@1692435)
      use gist tarball url that works for anonymous gists
      ([@isaacs](https://github.com/isaacs))
    * [`d5cf830`](npm/hosted-git-info@d5cf830)
      Do not allow invalid gist urls ([@isaacs](https://github.com/isaacs))
    * [`e518222`](npm/hosted-git-info@e518222)
      Use LRU cache to prevent unbounded memory consumption
      ([@iarna](https://github.com/iarna))

PR-URL: nodejs/node#29023
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
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.

None yet

3 participants