Failing tests - hashing and file size #210

Closed
wants to merge 2 commits into
from

Projects

None yet

6 participants

@btrask
Contributor
btrask commented Mar 25, 2013
  1. Hashing doesn't work on Node 0.10.0 or 0.8.22
  2. File size is zero on Node 0.10.0 (see #209)
@svnlto
Collaborator
svnlto commented Mar 25, 2013

thanks for those tests @btrask
However they seem to break the build.

@btrask
Contributor
btrask commented Mar 25, 2013

The build runs the tests and the tests fail. No surprises there. Making the tests pass is the hard part.

@svnlto
Collaborator
svnlto commented Mar 25, 2013

Oh I see, you've added them so there can be code written against them?
Would you mind adding code that supports those tests?

@btrask
Contributor
btrask commented Mar 25, 2013

I was hoping someone more familiar with the project would run with it. I've already worked around the issues in my own project by overriding onPart, which is more useful for my purposes anyway.

@egirshov egirshov was assigned Mar 25, 2013
@egirshov
Collaborator

@btrask, thanks for test cases. Hashing should be fixed here: #211, I'll take a look into file size problem next.

@net147
net147 commented Apr 13, 2013

Should be working now in node.js v0.10.4.

@carlos8f carlos8f referenced this pull request in amino/amino-deploy Apr 18, 2013
Closed

error: deployment #2

@tim-smart
Collaborator

This is working in Node >=0.10.4; in other versions the file._writeStream.write() call is not firing its callback, therefore not running this code: https://github.com/felixge/node-formidable/blob/afa2eb4a3e2922f0e84977b8d342f3601528ce77/lib/file.js#L55-L60

Funnily enough, Node v0.8 this code actually works; which makes me think this is actually an upstream Node bug.

Can anyone verify this for me?

@net147
net147 commented May 4, 2013

@tim-smart Node 0.10.4 fixes a regression which was causing this issue - see #209 for details.

@tim-smart tim-smart added a commit to tim-smart/node-formidable that referenced this pull request May 4, 2013
@tim-smart tim-smart Track the state of file write callbacks.
To make sure all the write callbacks are called before the write stream has
ended. Fixes #210

Signed-off-by: Tim Smart <tim@fostle.com>
e31e232
@tim-smart
Collaborator

Fixed this for v0.9 and earlier versions of v0.10. Just need to fix a regression in v0.8

@tim-smart tim-smart added a commit to tim-smart/node-formidable that referenced this pull request May 4, 2013
@tim-smart tim-smart Track the state of file write callbacks.
To make sure all the write callbacks are called before the write stream has
ended. Fixes #210

Signed-off-by: Tim Smart <tim@fostle.com>
1e0516e
@felixge
Owner
felixge commented May 4, 2013

@tim-smart thanks for working on this! The patch looks a bit complicated to me, and I think this could simplify it: tim-smart@1e0516e#commitcomment-3144964

@tim-smart
Collaborator

See the updated issue (#236) for the latest :)

@tim-smart tim-smart closed this Jun 19, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment