caolan / forms

An easy way to create, parse and validate forms in node.js
MIT License
1.01k stars 167 forks source link

tests aren’t executed and broken #189

Closed kmohrf closed 7 years ago

kmohrf commented 7 years ago

hi,

while working on a pull request and writing tests for it, I’ve noticed that the number of tests doesn’t increase when I add tests and run npm test or npm run test (which should be the same i guess).

the reason for this is that your tests are executed by calling node test/*.js. I don’t know if node changed it’s argument handling in more recent versions but it only seems to accept a single file for execution. this means that you only executed 357 out of 707 tests, when you ran npm test. the correct way to execute your tests is tape test/*.js.

when you change this you may notice that the tests are already broken. i’m not sure if this is dependent on the node version (i’m using node 6.7.0 on archlinux amd64) you’re using, because the tests don’t pass but there is nothing really wrong with your code. the problem is that you seem to rely on object key order for attributes and choices (funny enough my planned pull request actually involves indexed choices) which you can’t or to be exact which has only recently been defined as this article suggests. it might be possible that these tests worked in older node versions (I didn’t test that but would be interested) because object key order was different.

I’ve fixed the tests on this branch which i happily convert into a pull request if you want me to.

if i find the time i’ll be checking against some older node versions to verify the speculative parts concerning object-key-order and multiple script arguments.

kmohrf commented 7 years ago

ok… as nodejs.org provides statically build node versions this was easier than I anticipated.

the tests were not executed and broken in these versions: node-0.10.48, node-0.11.16, node-0.12.17, node-4.4.3 and node-4.6.1

ljharb commented 7 years ago

Thanks, great catch.

ljharb commented 7 years ago

Looks like I broke this in e6d17c9f, which means I'll have to recheck v1.1.3, v1.1.4, and v1.2.0

ljharb commented 7 years ago

Thankfully only v1.2.0 has test failures, so I won't have to backport any fixes.

ljharb commented 7 years ago

Closed via #190. Thanks!