Skip to content

Commit

Permalink
fix: bindings now error when closed during empty writes (#1872)
Browse files Browse the repository at this point in the history
Also fix tests not running!
  • Loading branch information
reconbot committed May 18, 2019
1 parent 524a272 commit 9d01492
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 68 deletions.
2 changes: 2 additions & 0 deletions packages/binding-abstract/binding-abstract.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ The in progress writes must error when the port is closed with an error object t

debug('write', buffer.length, 'bytes')
if (!this.isOpen) {
debug('write', 'error port is not open')

throw new Error('Port is not open')
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,17 @@ function disconnect(err) {
throw err || new Error('Unknown disconnection')
}

function shouldReject(promise, errType = Error, message = 'Should have rejected') {
return promise.then(
() => {
throw new Error(message)
},
err => {
assert.instanceOf(err, errType)
}
)
}

// All bindings are required to work with an "echo" firmware
// The echo firmware should respond with this data when it's
// ready to echo. This allows for remote device bootup.
Expand Down Expand Up @@ -156,22 +167,12 @@ function testBinding(bindingName, Binding, testPort) {
})
})

it('throws when not given a path', done => {
try {
binding.open('')
} catch (e) {
assert.instanceOf(e, TypeError)
done()
}
it('throws when not given a path', async () => {
await shouldReject(binding.open(''), TypeError)
})

it('throws when not given options', done => {
try {
binding.open('COMBAD')
} catch (e) {
assert.instanceOf(e, TypeError)
done()
}
it('throws when not given options', async () => {
await shouldReject(binding.open('COMBAD'), TypeError)
})

if (!testPort) {
Expand Down Expand Up @@ -278,15 +279,9 @@ function testBinding(bindingName, Binding, testPort) {
})

describe('#update', () => {
it('throws when not given an object', done => {
it('throws when not given an object', async () => {
const binding = new Binding({ disconnect })

try {
binding.update()
} catch (e) {
assert.instanceOf(e, TypeError)
done()
}
await shouldReject(binding.update(), TypeError)
})

it('errors asynchronously when not open', done => {
Expand Down Expand Up @@ -315,53 +310,47 @@ function testBinding(bindingName, Binding, testPort) {

afterEach(() => binding.close())

it('throws errors when updating nothing', done => {
try {
binding.update({})
} catch (err) {
assert.instanceOf(err, Error)
done()
}
it('throws errors when updating nothing', async () => {
await shouldReject(binding.update({}), Error)
})

it('errors when not called with options', done => {
try {
binding.set(() => {})
} catch (e) {
assert.instanceOf(e, Error)
done()
}
it('errors when not called with options', async () => {
await shouldReject(binding.set(() => {}), Error)
})

it('updates baudRate', () => {
return binding.update({ baudRate: 57600 })
})
})

describe('#write', () => {
describe.only('#write', () => {
it('errors asynchronously when not open', done => {
const binding = new Binding({
disconnect,
})
let noZalgo = false
binding.write(Buffer.from([])).catch(err => {
assert.instanceOf(err, Error)
assert(noZalgo)
done()
})
binding
.write(Buffer.from([]))
.then(
data => {
console.log({ data })
throw new Error('Should have errored')
},
err => {
assert.instanceOf(err, Error)
assert(noZalgo)
done()
}
)
.catch(done)
noZalgo = true
})

it('throws when not given a buffer', done => {
it('throws when not given a buffer', async () => {
const binding = new Binding({
disconnect,
})
try {
binding.write(null)
} catch (e) {
assert.instanceOf(e, TypeError)
done()
}
await shouldReject(binding.write(null), TypeError)
})

if (!testPort) {
Expand Down Expand Up @@ -494,16 +483,11 @@ function testBinding(bindingName, Binding, testPort) {
noZalgo = true
})

it('throws when not called with options', done => {
it('throws when not called with options', async () => {
const binding = new Binding({
disconnect,
})
try {
binding.set(() => {})
} catch (e) {
assert.instanceOf(e, TypeError)
done()
}
await shouldReject(binding.set(() => {}), TypeError)
})

if (!testPort) {
Expand Down
10 changes: 6 additions & 4 deletions packages/bindings/lib/darwin.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,14 @@ class DarwinBinding extends AbstractBinding {
}

async write(buffer) {
if (buffer.length === 0) {
return
}
this.writeOperation = super
.write(buffer)
.then(() => unixWrite.call(this, buffer))
.then(() => {
if (buffer.length === 0) {
return
}
return unixWrite.call(this, buffer)
})
.then(() => {
this.writeOperation = null
})
Expand Down
10 changes: 6 additions & 4 deletions packages/bindings/lib/linux.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,14 @@ class LinuxBinding extends AbstractBinding {
}

async write(buffer) {
if (buffer.length === 0) {
return
}
this.writeOperation = super
.write(buffer)
.then(() => unixWrite.call(this, buffer))
.then(() => {
if (buffer.length === 0) {
return
}
return unixWrite.call(this, buffer)
})
.then(() => {
this.writeOperation = null
})
Expand Down
10 changes: 6 additions & 4 deletions packages/bindings/lib/win32.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,14 @@ class WindowsBinding extends AbstractBinding {
}

async write(buffer) {
if (buffer.length === 0) {
return
}
this.writeOperation = super
.write(buffer)
.then(() => asyncWrite(this.fd, buffer))
.then(() => {
if (buffer.length === 0) {
return
}
return asyncWrite(this.fd, buffer)
})
.then(() => {
this.writeOperation = null
})
Expand Down

0 comments on commit 9d01492

Please sign in to comment.