hapijs / crumb

CSRF crumb generation and validation for hapi
Other
171 stars 50 forks source link

Tests fail on v6.0.2 #86

Closed BCook98 closed 8 years ago

BCook98 commented 8 years ago

Running the tests on master fail, giving me the following error. I get the same error on both node v4 and v6, but not on the tag v6.0.1.

  this.push is not a function

  at TestStream._read (crumb/test/index.js:146:46)
  at Readable.read (_stream_readable.js:336:10)
  at resume_ (_stream_readable.js:733:12)
  at nextTickCallbackWith2Args (node.js:442:9)
  at process._tickDomainCallback (node.js:397:17)

I am not sure why the tests on Travis passed yet are failing for me, but I have attempted this in multiple environments. It looks like this is never assigned as arrow functions are used all of the way up to the root.

Any ideas?

Marsup commented 8 years ago

This line is obviously wrong, it should be a normal function. Not sure why travis thinks it's ok.

stongo commented 8 years ago

@BCook98 I can't reproduce locally. What environment are you running the test in?

blastoise:crumb stongo$ node --version
v4.2.1
blastoise:crumb stongo$ npm test

> crumb@6.0.2 test /Users/stongo/dev/crumb
> lab -r console -t 100 -a code -L

  ...........

11 tests complete
Test duration: 291 ms
Assertions count: 70 (verbosity: 6.36)
No global variable leaks detected
Coverage: 100.00%
Linting results: No issues
BCook98 commented 8 years ago

@stongo Super weird, Same error on both latest Node v4 and v6. I am running OSX (zsh and n), but I get the same thing on my CentOS VM (bash and nvm).

~/hapijs/crumb (master)$ node -v
v4.2.1
~/hapijs/crumb (master)$ npm test

> crumb@6.0.2 test /Users/brandoncook/hapijs/crumb
> lab -r console -t 100 -a code -L

  x..........

Test script errors:

this.push is not a function
      at TestStream._read (/Users/brandoncook/hapijs/crumb/test/index.js:146:46)
      at Readable.read (_stream_readable.js:328:10)
      at resume_ (_stream_readable.js:718:12)
      at doNTCallback2 (node.js:439:9)
      at process._tickDomainCallback (node.js:394:17)

There were 1 test script error(s).

Failed tests:

  1) Crumb returns view with crumb:

      this.push is not a function

      at TestStream._read (/Users/brandoncook/hapijs/crumb/test/index.js:146:46)
      at Readable.read (_stream_readable.js:328:10)
      at resume_ (_stream_readable.js:718:12)
      at doNTCallback2 (node.js:439:9)
      at process._tickDomainCallback (node.js:394:17)

1 of 11 tests failed
Test duration: 459 ms
Assertions count: 62 (verbosity: 5.64)
No global variable leaks detected
Coverage: 99.20% (1/125)
lib/index.js missing coverage on line(s): 150
Code coverage below threshold: 99.20 < 100
Linting results: No issues

npm ERR! Test failed.  See above for more details.
Marsup commented 8 years ago

Also fails locally for me. This code should clearly fail.

stongo commented 8 years ago

Clearly? "When readable._read() is called, if data is available from the resource, the implementation should begin pushing that data into the read queue using the this.push(dataChunk) method. _read() should continue reading from the resource and pushing data until readable.push() returns false."

Marsup commented 8 years ago

Yes, except the this is in no way referring to the stream, that's just impossible.

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.