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

add random-access-test as dependency. #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rhodey
Copy link

@rhodey rhodey commented Jan 11, 2020

patch empty read issue.
patch destroy w/o callback issue.
bump version 1.0.1

notice that size and content test suites are skipped. at this time they fail and I don't have the time to try and fix that part right now.

Copy link
Collaborator

@martinheidegger martinheidegger left a comment

Choose a reason for hiding this comment

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

Thank you for adding this, please remove the version bump, the .gitignore and package-lock.json.

package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "random-access-pause-wrapper",
"version": "1.0.0",
"version": "1.0.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please do not bump the version in pull requests.

2. patch empty read issue.
3. patch destroy w/o callback issue.
@@ -33,7 +35,11 @@ Object.assign(RASPauseWrapper.prototype, {
})
},
read: function (offset, size, cb) {
this._onResumeCb(cb, function (cbProxy) { this._proxied.read(offset, size, cbProxy) })
if (size === 0) {
this._onResumeCb(cb, function (cbProxy) { cbProxy(null, Buffer.alloc(0)) })
Copy link
Collaborator

Choose a reason for hiding this comment

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

Even a read of size 0 should be triggered in the _proxied instance as oder side effects (such as logging) might be triggered. Why did you add this optimization?

Copy link
Author

Choose a reason for hiding this comment

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

Hi thank you for taking time to review, from my memory when I was running the test suite there was a zero-length-read test which failed and so I added this as a quick hard-to-mess-up patch. let me review this now gimmie a few minutes 👍

Copy link
Author

Choose a reason for hiding this comment

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

ok yes so I get two failed tests without this, here is my npm run test log:

npm run test

> [email protected] test /home/rhodey/dev/dsp/radiowitness.dat/lib/js/random-access-pause-wrapper
> standard && tape *.test.js

TAP version 13
# basic listening
ok 1 By default no listener is added
ok 2 Just because a listener is added to the target doesnt mean there is a listener on the source
ok 3 Return type of 
ok 4 There should be a listener on the target, after adding one to the proxy
ok 5 target event is passed to the proxy
# once
ok 6 Result of once should be the proxy instance
ok 7 Should have listener after once added
ok 8 event called once.
ok 9 data passed through
ok 10 Should not have listener on target after once executed
ok 11 Should not have listener on proxy after once executed
# prepend once
ok 12 Result of prepend once is proxy instance
ok 13 event called once, before the listeners on proxy, after the listeners on target
ok 14 data passed through
ok 15 should be equal
# remove all listeners
ok 16 Result of removeAllListeners is proxy instance
ok 17 target listener is still called
# own events
ok 18 result of on is proxy instance
ok 19 target called with world
ok 20 proxy called with mundo, target not passed to proxy
# Pause order events
ok 21 The instance is marked as pausable
ok 22 Initially not paused
ok 23 Execution of pause is successful
ok 24 Calling pause makes it paused
ok 25 Pausing when paused doesnt work
ok 26 Calling once again doesnt change it
ok 27 Resume works when paused
ok 28 Calling resume unpauses it
ok 29 Multiple resumes have no effect
ok 30 Calling resume unpauses it
ok 31 Proper event order
# ._onResume execution order
ok 32 should be equivalent
# ._onResumeCb
ok 33 passed-in cb shouldnt match initial cb
ok 34 wrapper should be set as scope
ok 35 wrapper should be set as scope
ok 36 error properly propagated
ok 37 data properly propagated
ok 38 should be equivalent
# events are not emitted on pause
ok 39 should be equal
ok 40 should be equivalent
# Closing the wrapper stops all events
ok 41 even though private "_closed" should be predictable
ok 42 should be equivalent
# pause on resume
ok 43 should be equivalent
# write and read
ok 44 no error
ok 45 no error
ok 46 should be equivalent
# read empty
not ok 47 no error
  ---
    operator: error
    expected: |-
      undefined
    actual: |-
      { [Error: ENOENT: no such file or directory, open '/tmp/random-pause-test/create-empty.txt'] errno: -2, code: 'ENOENT', syscall: 'open', path: '/tmp/random-pause-test/create-empty.txt' }
    at: RASPauseWrapper.<anonymous> (/home/rhodey/dev/dsp/radiowitness.dat/lib/js/random-access-pause-wrapper/node_modules/random-access-test/index.js:23:11)
    stack: |-
      Error: ENOENT: no such file or directory, open '/tmp/random-pause-test/create-empty.txt'
  ...
not ok 48 empty buffer
  ---
    operator: deepEqual
    expected: <Buffer >
    actual:   undefined
    at: RASPauseWrapper.<anonymous> (/home/rhodey/dev/dsp/radiowitness.dat/lib/js/random-access-pause-wrapper/node_modules/random-access-test/index.js:24:11)
    stack: |-
      Error: empty buffer
          at Test.assert [as _assert] (/home/rhodey/dev/dsp/radiowitness.dat/lib/js/random-access-pause-wrapper/node_modules/tape/lib/test.js:224:54)
          at Test.bound [as _assert] (/home/rhodey/dev/dsp/radiowitness.dat/lib/js/random-access-pause-wrapper/node_modules/tape/lib/test.js:76:32)
          at Test.tapeDeepEqual (/home/rhodey/dev/dsp/radiowitness.dat/lib/js/random-access-pause-wrapper/node_modules/tape/lib/test.js:421:10)
          at Test.bound [as same] (/home/rhodey/dev/dsp/radiowitness.dat/lib/js/random-access-pause-wrapper/node_modules/tape/lib/test.js:76:32)
          at RASPauseWrapper.<anonymous> (/home/rhodey/dev/dsp/radiowitness.dat/lib/js/random-access-pause-wrapper/node_modules/random-access-test/index.js:24:11)
          at RASPauseWrapper.<anonymous> (/home/rhodey/dev/dsp/radiowitness.dat/lib/js/random-access-pause-wrapper/PauseWrapper.js:39:14)
          at RASPauseWrapper._onResume (/home/rhodey/dev/dsp/radiowitness.dat/lib/js/random-access-pause-wrapper/PauseWrapper.js:31:13)
          at Request._callback (/home/rhodey/dev/dsp/radiowitness.dat/lib/js/random-access-pause-wrapper/PauseWrapper.js:37:15)
          at Request.callback (/home/rhodey/dev/dsp/radiowitness.dat/lib/js/random-access-pause-wrapper/node_modules/random-access-storage/index.js:161:8)
          at nextTickCallback (/home/rhodey/dev/dsp/radiowitness.dat/lib/js/random-access-pause-wrapper/node_modules/random-access-storage/index.js:249:7)
  ...
# read range > file
ok 49 not satisfiable
# read range > file with data
ok 50 no error
ok 51 not satisfiable
# random access write and read
ok 52 no error
ok 53 no error
ok 54 no error
ok 55 should be equivalent
ok 56 no error
ok 57 should be equivalent
ok 58 no error
ok 59 should be equivalent
# re-open
ok 60 no error
ok 61 no error
ok 62 should be equivalent
# truncate with size
ok 63 no error
ok 64 should be equivalent
# mkdir path
ok 65 no error
ok 66 no error
ok 67 should be equivalent
# write/read big chunks
ok 68 no error
ok 69 no error
ok 70 no error
ok 71 should be equivalent
ok 72 no error
ok 73 should be equivalent
# del
ok 74 no error
ok 75 no error
ok 76 should be equivalent
ok 77 no error
ok 78 no error
ok 79 inplace del, same size
ok 80 no error
ok 81 no error
ok 82 should be equivalent
# open and close many times
ok 83 no error
ok 84 no error
# write and read
ok 85 no error
ok 86 no error
ok 87 should be equivalent
# read empty
ok 88 no error
ok 89 empty buffer
# read range > file
ok 90 not satisfiable
# random access write and read
ok 91 no error
ok 92 no error
ok 93 no error
ok 94 should be equivalent
ok 95 no error
ok 96 should be equivalent
ok 97 no error
ok 98 should be equivalent
# buffer constructor
ok 99 null
ok 100 should be equivalent
# pause after read
ok 101 should be equal
ok 102 should be equal
ok 103 null
ok 104 should be equivalent
ok 105 null
ok 106 should be equivalent
ok 107 should be equivalent
# open/close/destroy
ok 108 file.opened == false
ok 109 file.closed == false
ok 110 file.destroyed == false
ok 111 mem.opened == false
ok 112 mem.closed == false
ok 113 mem.destroyed == false
ok 114 file.opened == false
ok 115 file.closed == false
ok 116 file.destroyed == false
ok 117 mem.opened == false
ok 118 mem.closed == false
ok 119 mem.destroyed == false
ok 120 open successful
ok 121 file.opened == true
ok 122 file.closed == false
ok 123 file.destroyed == false
ok 124 mem.opened == true
ok 125 mem.closed == false
ok 126 mem.destroyed == false
ok 127 close successful
ok 128 file.opened == true
ok 129 file.closed == true
ok 130 file.destroyed == false
ok 131 mem.opened == true
ok 132 mem.closed == true
ok 133 mem.destroyed == false
ok 134 destroy successful
ok 135 file.opened == true
ok 136 file.closed == true
ok 137 file.destroyed == true
ok 138 mem.opened == true
ok 139 mem.closed == true
ok 140 mem.destroyed == true
# open/close while pause
ok 141 should be equal
ok 142 now it is closed
ok 143 close works while pause
ok 144 should be equivalent

1..144
# tests 144
# pass  142
# fail  2

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] test: `standard && tape *.test.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] test script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/rhodey/.npm/_logs/2020-01-16T04_33_25_215Z-debug.log

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.

2 participants