Change testing suite to follow lisk standards - Closes #448#437
Change testing suite to follow lisk standards - Closes #448#437willclarktech merged 6 commits into1.0.0from
Conversation
d866859 to
2dd5643
Compare
|
|
||
| new Assertion(obj).to.be.an('array'); | ||
| let result = false; | ||
| obj.forEach(val => { |
There was a problem hiding this comment.
Prefer using const and .some.
| // See https://github.com/shouldjs/should.js/issues/41 | ||
| Object.defineProperty(global, 'should', { value: should }); | ||
|
|
||
| [sinonChai, chaiAsPromised].forEach(plugin => chai.use(plugin)); |
There was a problem hiding this comment.
Might be neater to use just .forEach(chai.use)?
| this.assert( | ||
| result, | ||
| 'expected #{this} to match at least once', | ||
| 'expected #{this} to not to match', |
| const expected = Buffer.from(actual, 'hex').toString('hex'); | ||
| this.assert( | ||
| expected === actual, | ||
| 'expected #{this} to be a hexString', |
There was a problem hiding this comment.
We don't need camelcase in the user-facing message.
| export function itShouldResolveToTheAPIResponse() { | ||
| const { returnValue, apiResponse } = this.test.ctx; | ||
| return returnValue.should.be.fulfilledWith(apiResponse); | ||
| return returnValue.should.be.eventually.eql(apiResponse); |
There was a problem hiding this comment.
.should.eventually.eql
There was a problem hiding this comment.
I think we need be verb in natural language?
| export function itShouldResolveToTheConfig() { | ||
| const { returnValue, config } = this.test.ctx; | ||
| return returnValue.should.be.fulfilledWith(config); | ||
| return returnValue.should.be.eventually.eql(config); |
| return testFunction.should | ||
| .throw() | ||
| .and.has.property('message') | ||
| .which.include(message); |
There was a problem hiding this comment.
All these testFunction.should.throw()s seem redundant. It's awkward, but I think it would be better to store that in a variable and make further assertions against it.
| return testFunction.should | ||
| .throw() | ||
| .and.has.property('message') | ||
| .which.include(message); |
There was a problem hiding this comment.
Let's extract the common functionality into a helper function, so these then steps can be cleaner at least. Can we even add a method to chai?
| err.should.be.instanceOf(Error); | ||
| err.should.have.property('name').which.equal('FileSystemError'); | ||
| return err.should.have.property('message').which.include(message); | ||
| }); |
There was a problem hiding this comment.
Similar suggestion about adding a method to chai.
There was a problem hiding this comment.
great idea! it's much cleaner.
I will add customError check function
| export function itShouldReturnNull() { | ||
| const { returnValue } = this.test.ctx; | ||
| return should(returnValue).be.null(); | ||
| return should.equal(returnValue, null); |
There was a problem hiding this comment.
This is another downside to switching to chai.
There was a problem hiding this comment.
But I guess it'll be solved when we switch to expect.
There was a problem hiding this comment.
yes, it will.
I almost tried to port except to global, too... lol
55147d3 to
c06eecb
Compare
c06eecb to
5182fed
Compare
willclarktech
left a comment
There was a problem hiding this comment.
- README.md then step should be updated.
- Prettier should be run on all files.
| export function itShouldResolveToThePassphrase() { | ||
| const { returnValue, passphrase } = this.test.ctx; | ||
| return returnValue.should.be.fulfilledWith(passphrase); | ||
| return returnValue.should.eventually.eql(passphrase); |
There was a problem hiding this comment.
Prefer .equal where possible.
| const message = getFirstQuotedString(this.test.title); | ||
| return returnValue.should.be.rejectedWith(new FileSystemError(message)); | ||
| return returnValue.should.be.rejected.then(err => { | ||
| return err.should.be.customError(new FileSystemError(message)); |
There was a problem hiding this comment.
Prefer direct return without function body.
| const message = getFirstQuotedString(this.test.title); | ||
| return returnValue.should.be.rejectedWith(new ValidationError(message)); | ||
| return returnValue.should.be.rejected.then(err => { | ||
| return err.should.be.customError(new ValidationError(message)); |
Closes #448