-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
avoid throwing for flow control #1924
Changes from 7 commits
080c1fa
6e2bdad
eadaa09
4bafab2
5cdeb06
ecf2c31
172c19a
8c8bd82
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,37 @@ | ||
import assert from 'assert' | ||
import { assertNodeEngineVersion } from './assert_node_engine_version' | ||
import { expect } from 'chai' | ||
import * as sinon from 'sinon' | ||
import { validateNodeEngineVersion } from './assert_node_engine_version' | ||
|
||
describe(assertNodeEngineVersion.name, () => { | ||
it('fails when the version is lower than specified in package.json', () => { | ||
assert.throws(() => | ||
assertNodeEngineVersion('v11.0.0', () => ({ | ||
engines: { | ||
node: '>=12', | ||
}, | ||
})) | ||
describe(validateNodeEngineVersion.name, () => { | ||
it('returns an error message when the version is lower than specified in package.json', () => { | ||
// Arrange | ||
const errorSpy = sinon.spy() | ||
|
||
// Act | ||
validateNodeEngineVersion('v11.0.0', errorSpy, () => ({ | ||
engines: { | ||
node: '>=12', | ||
}, | ||
})) | ||
|
||
// Assert | ||
expect(errorSpy).to.have.been.calledOnceWith( | ||
'Cucumber can only run on Node.js versions >=12. This Node.js version is v11.0.0' | ||
) | ||
}) | ||
|
||
it('passes when the version is greater than specified in package.json', () => { | ||
assertNodeEngineVersion('v17.0.0') | ||
it('returns null when the version is greater than specified in package.json', () => { | ||
// Arrange | ||
const errorSpy = sinon.spy() | ||
|
||
// Act | ||
validateNodeEngineVersion('v17.0.0', errorSpy, () => ({ | ||
engines: { | ||
node: '>=12', | ||
}, | ||
})) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Think its better to hardcode the engines here as well so this doesn't suddenly fail when we no longer support version 17 (probably a little ways off) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💯, I missed that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh true, the list right now is 12, 14, 16, 17 |
||
|
||
// Assert | ||
expect(errorSpy).not.to.have.been.called() | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realised we should have changed these example descriptions too: we're not returning anything anymore.