split by size does not work

[delprofundo]

Split by size (say at 1000000 bytes)

using this:

async function splitFile( filename, chunkSize ) {
  return new Promise(( resolve, reject ) => {
    split.splitFileBySize( __dirname + filename, chunkSize )
      .then(names => {
        resolve( names )
      })
      .catch(err => {
        console.log("NEEEEEEEERRRRRR : ", err);
        reject( err );
      })
  })
} // end splitFile

does this

-rw-r--r--  1  staff   5.0M Oct  2  2017 abc123.mp4
-rw-r--r--  1  staff   977K Oct 25 11:35 abc123.mp4.sf-part1
-rw-r--r--  1  staff   4.0M Oct 25 11:35 abc123.mp4.sf-part2
-rw-r--r--  1  staff   3.1M Oct 25 11:35 abc123.mp4.sf-part3
-rw-r--r--  1  staff   2.1M Oct 25 11:35 abc123.mp4.sf-part4
-rw-r--r--  1  staff   1.2M Oct 25 11:35 abc123.mp4.sf-part5
-rw-r--r--  1  staff   232K Oct 25 11:35 abc123.mp4.sf-part6

[snowww1014]

Same problem.

[tomvlk]

Can you provide me with your parameters of the chunk size, the node version and the package version? @delprofundo @snowww1014

[AleksMeshkov]

Hello @tomvlk !

I'm digging through the code and see this:

The lastSplitSize value is a negative number. Is it ok?

There is one more bug I think I found (line 123):

This condition will be never executed because i will always be less the parts value (have a look and the loop condition on line 111: for (var i = 0; i < parts; i ++))

[AleksMeshkov]

@tomvlk I created a PR (https://github.com/tomvlk/node-split-file/pull/31) But I'm not sure if this code is necessary at all.

Both tests show the same size with and without the fix. What do you think?

[tomvlk]

Well, to be honest, the tests are very limited and doesn't include enough test cases to be strict and should be improved, this is I think why the tests didn't catch this bug.

I looked at the code and indeed, this is the exact bug, I will merge your pull request and submit a new version to NPM right away.

Thank you @AleksMeshkov

[tomvlk]

Ah, @AleksMeshkov I just found out that there is another issue when you have a decimal maxSize. To handle this I round the input maxSize. Also, I simplified the code a bit.

[AleksMeshkov]

@tomvlk I created a PR (#31) But I'm not sure if this code is necessary at all.

Both tests show the same size with and without the fix. What do you think?

@tomvlk and what about this? Do you think we need this logic at all? I mean calculation of the last chunk size. Without this, code works the same.

[tomvlk]

@AleksMeshkov I updated the code because calculating is not necessary. The last chund end should be the total size of the file and that is how it's been implemented right now.

[delprofundo]

awesome. im building a test suite this weekend that could use it.