conform() with Sets added too many items #45
Conversation
mattbishop
changed the title
mattbishop
force-pushed
the
sets-broken
branch
2 times, most recently
from
d0b315a to
0471cdd
Compare
mattbishop
force-pushed
the
sets-broken
branch
from
0471cdd to
26b118d
Compare
| @@ -6,6 +6,12 @@ import { explainData } from "../../index"; | |||
| import { invalid, count, minCount, maxCount } from "../../lib/symbols"; | |||
|
|
|||
| describe("collection", () => { | |||
| describe("factory", () => { | |||
There was a problem hiding this comment.
Coveralls was complaining about reduced test coverage so I added a new test to increase coverage.
| return invalid; | ||
| } | ||
| ret[key] = conformed; | ||
| const spreadValue = p.array(value) ? value : [...value]; |
There was a problem hiding this comment.
for / of was adding in a length key, which was the source of this bug. Using spread operator converts the Set to an array with simpler map / filter functions.
| "eslint": "^4.4.1", | ||
| "eslint-plugin-prettier": "^2.1.2", | ||
| "mocha": "^3.1.2", | ||
| "chai": "^4.1.1", |
There was a problem hiding this comment.
Set testing was broken in old unit testing libs. I upgraded these together to the latest version to be safe.
|
|
||
| expect(spec.conform(new Set([1, 2])), "set - count, happy").to.be | ||
| .a("Set") | ||
| .that.contains(1, 2) |
There was a problem hiding this comment.
deep.equal failed for Sets. It considered the two Sets equal, even though one has 3 elements and the other has 2.
There was a problem hiding this comment.
👍 Related -> chaijs/deep-eql#4
|
LGTM |
|
@prayerslayer any comments on this PR? |
|
Will push a new version as well |
js.spec's Set handling is broken WRT conform() output. The returned Set includes an extra 'length' field.
This PR fixes how conform assembles the return value. It upgrades the unit testing to chai 4 to gain improved Set assertions.