test: improve test coverage of internal/blob #41513

kuriyosh · 2022-01-14T07:38:07Z

This improves a test coverage in lib/internal/blob
ref: https://coverage.nodejs.org/coverage-74b9baa4265a8f0d/lib/internal/blob.js.html

This added following tests.

executing inspect to Blob with depth option
creating new Blob instance with over size argument
causing ERR_INVALID_THIS errors in some methods

nodejs-github-bot · 2022-01-18T02:28:48Z

CI: https://ci.nodejs.org/job/node-test-pull-request/41978/

nodejs-github-bot · 2022-01-18T16:36:11Z

CI: https://ci.nodejs.org/job/node-test-pull-request/41994/

aduh95 · 2022-01-18T17:58:58Z

There are several related failures in the CI test results:

AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected

+ Comparison {}
- Comparison {
-   code: 'ERR_BUFFER_TOO_LARGE'
- }
    at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux/test/parallel/test-blob.js:234:10)
    at Module._compile (node:internal/modules/cjs/loader:1097:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1149:10)
    at Module.load (node:internal/modules/cjs/loader:975:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:77:12)
    at node:internal/main/run_main_module:17:47 {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: RangeError: Array buffer allocation failed
      at new ArrayBuffer (<anonymous>)
      at new Uint8Array (<anonymous>)
      at assert.throws.code (/home/iojs/build/workspace/node-test-commit-linux/test/parallel/test-blob.js:234:33)
      at getActual (node:assert:756:5)
      at Function.throws (node:assert:902:24)
      at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux/test/parallel/test-blob.js:234:10)
      at Module._compile (node:internal/modules/cjs/loader:1097:14)
      at Object.Module._extensions..js (node:internal/modules/cjs/loader:1149:10)
      at Module.load (node:internal/modules/cjs/loader:975:32)
      at Function.Module._load (node:internal/modules/cjs/loader:822:12),
  expected: { code: 'ERR_BUFFER_TOO_LARGE' },
  operator: 'throws'
}

Can you take a look please?

aduh95 · 2022-01-21T10:27:54Z

I'm not sure this is a correct fix, the error was RangeError: Array buffer allocation failed, increasing the size would probably not help! A "workaround" would be to skip the test if the memory constraints on the platform doesn't let us run the test, as it's done in some other tests, e.g.

node/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-at-max.js

Lines 16 to 22 in 7d453ff

    
           try { 
        
             buf = Buffer.allocUnsafe(kStringMaxLength); 
        
           } catch (e) { 
        
             // If the exception is not due to memory confinement then rethrow it. 
        
             if (e.message !== 'Array buffer allocation failed') throw (e); 
        
             common.skip(skipMessage); 
        
           }

kuriyosh · 2022-01-28T00:53:41Z

@aduh95
I'm sorry to reply late.
Thank you! I didn't know the correct handling in this case, so I took an approach to avoid it by reducing the length of each array passed to the Buffer argument.
I will fix it by the method you suggested!

test/parallel/test-blob.js

nodejs-github-bot · 2022-01-30T15:21:52Z

CI: https://ci.nodejs.org/job/node-test-pull-request/42243/

nodejs-github-bot · 2022-04-08T10:13:51Z

CI: https://ci.nodejs.org/job/node-test-pull-request/43391/

nodejs-github-bot · 2022-04-08T13:38:13Z

CI: https://ci.nodejs.org/job/node-test-pull-request/43395/

nodejs-github-bot · 2022-04-14T21:59:21Z

Commit Queue failed

- Loading data for nodejs/node/pull/41513
✔  Done loading data for nodejs/node/pull/41513
----------------------------------- PR info ------------------------------------
Title      test: improve test coverage of internal/blob (#41513)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     kuriyosh:test-blob-coverage -> nodejs:master
Labels     test, author ready, needs-ci, commit-queue-squash
Commits    7
 - test: improve test coverage of internal/blob
 - test: over size array in some platforms
 - fix: skip oversized buffer test for memory limitation
 - fix: remove parens in throw statement
 - test: fix mustcall error
 - test: skip invalid array length
 - test: skip this test in freeBSD env
Committers 1
 - Yoshiki Kurihara 
PR-URL: https://github.com/nodejs/node/pull/41513
Refs: https://coverage.nodejs.org/coverage-74b9baa4265a8f0d/lib/internal/blob.js.html
Reviewed-By: Colin Ihrig 
Reviewed-By: Antoine du Hamel 
Reviewed-By: James M Snell 
Reviewed-By: Daijiro Wachi 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/41513
Refs: https://coverage.nodejs.org/coverage-74b9baa4265a8f0d/lib/internal/blob.js.html
Reviewed-By: Colin Ihrig 
Reviewed-By: Antoine du Hamel 
Reviewed-By: James M Snell 
Reviewed-By: Daijiro Wachi 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - test: skip this test in freeBSD env
   ℹ  This PR was created on Fri, 14 Jan 2022 07:38:07 GMT
   ✔  Approvals: 4
   ✔  - Colin Ihrig (@cjihrig) (TSC): https://github.com/nodejs/node/pull/41513#pullrequestreview-853117382
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/41513#pullrequestreview-907913460
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/41513#pullrequestreview-907915108
   ✔  - Daijiro Wachi (@watilde): https://github.com/nodejs/node/pull/41513#pullrequestreview-912533681
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-04-08T13:38:13Z: https://ci.nodejs.org/job/node-test-pull-request/43395/
- Querying data for job/node-test-pull-request/43395/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/2169804667

nodejs-github-bot · 2022-04-14T22:19:59Z

Landed in 1285ac1

PR-URL: nodejs#41513 Refs: https://coverage.nodejs.org/coverage-74b9baa4265a8f0d/lib/internal/blob.js.html Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>

PR-URL: #41513 Refs: https://coverage.nodejs.org/coverage-74b9baa4265a8f0d/lib/internal/blob.js.html Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>

PR-URL: nodejs/node#41513 Refs: https://coverage.nodejs.org/coverage-74b9baa4265a8f0d/lib/internal/blob.js.html Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>

test: improve test coverage of internal/blob

15b4cd4

nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Jan 14, 2022

cjihrig approved these changes Jan 14, 2022

View reviewed changes

jasnell previously approved these changes Jan 17, 2022

View reviewed changes

Ayase-252 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jan 18, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 18, 2022

aduh95 removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 18, 2022

This was referenced Jan 19, 2022

CI Reliability 2022-01-19 nodejs/reliability#176

Open

CI Reliability 2022-01-20 nodejs/reliability#177

Open

CI Reliability 2022-01-21 nodejs/reliability#178

Open

test: over size array in some platforms

8b1c102

This was referenced Jan 22, 2022

CI Reliability 2022-01-22 nodejs/reliability#179

Open

CI Reliability 2022-01-23 nodejs/reliability#180

Open

fix: skip oversized buffer test for memory limitation

23d0c45

aduh95 reviewed Jan 28, 2022

View reviewed changes

test/parallel/test-blob.js Outdated Show resolved Hide resolved

fix: remove parens in throw statement

24fa55e

aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jan 30, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 30, 2022

This was referenced Jan 31, 2022

CI Reliability 2022-01-31 nodejs/reliability#188

Open

CI Reliability 2022-02-01 nodejs/reliability#189

Open

CI Reliability 2022-02-02 nodejs/reliability#190

Open

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 8, 2022

targos mentioned this pull request Apr 13, 2022

Investigate flaky crash in test/pummel/test-repl-empty-maybelocal-crash.js on freebsd #42719

Closed

github-actions bot mentioned this pull request Apr 14, 2022

CI Reliability 2022-04-14 nodejs/reliability#251

Open

24 tasks

aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 14, 2022

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Apr 14, 2022

aduh95 approved these changes Apr 14, 2022

View reviewed changes

aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Apr 14, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 14, 2022

nodejs-github-bot merged commit 1285ac1 into nodejs:master Apr 14, 2022

targos mentioned this pull request May 2, 2022

v18.1.0 release proposal #42943

Merged

juanarbol mentioned this pull request May 31, 2022

2022-05-XX, Version 16.16.0 'Gallium' (LTS) #43268

Closed

targos mentioned this pull request Aug 3, 2022

v16.17.0 release proposal #44098

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: improve test coverage of internal/blob #41513

test: improve test coverage of internal/blob #41513

kuriyosh commented Jan 14, 2022

nodejs-github-bot commented Jan 18, 2022

nodejs-github-bot commented Jan 18, 2022

aduh95 commented Jan 18, 2022 •

edited

Loading

aduh95 commented Jan 21, 2022

kuriyosh commented Jan 28, 2022

nodejs-github-bot commented Jan 30, 2022

nodejs-github-bot commented Apr 8, 2022

nodejs-github-bot commented Apr 8, 2022

nodejs-github-bot commented Apr 14, 2022

nodejs-github-bot commented Apr 14, 2022

test: improve test coverage of internal/blob #41513

test: improve test coverage of internal/blob #41513

Conversation

kuriyosh commented Jan 14, 2022

nodejs-github-bot commented Jan 18, 2022

nodejs-github-bot commented Jan 18, 2022

aduh95 commented Jan 18, 2022 • edited Loading

aduh95 commented Jan 21, 2022

kuriyosh commented Jan 28, 2022

nodejs-github-bot commented Jan 30, 2022

nodejs-github-bot commented Apr 8, 2022

nodejs-github-bot commented Apr 8, 2022

nodejs-github-bot commented Apr 14, 2022

nodejs-github-bot commented Apr 14, 2022

aduh95 commented Jan 18, 2022 •

edited

Loading