Skip to content

fix: spawn options are processed#115

Merged
q2s2t merged 3 commits intoq2s2t:masterfrom
jliben:fix-spawn-options
Oct 30, 2022
Merged

fix: spawn options are processed#115
q2s2t merged 3 commits intoq2s2t:masterfrom
jliben:fix-spawn-options

Conversation

@jliben
Copy link
Contributor

@jliben jliben commented Feb 22, 2022

While using this nice lib to compress a pretty big folder, I realized that the child_process is spawned as a background process.
On my (windows) machine, this leads to the 7za child to continue running when I interrupt the node.js program that spawned it.

I think it would make sense to default the detached option of the child_process to false, however, in order to keep background compatibility, I tried to use the documented $spawnOptions, like so:

      const compressStream = add(
        path.resolve(RELEASE_DIR, "archive.7z"),
        ".",
        {
          $bin: sevenBin.path7za,
          $progress: true,
          $spawnOptions: {
            detached: false,
          },
        }
      )

Just realised it was not processed, so I'm submitting this little fix to support it.

Basic unit tests added as well, I could not really figure out how to write more advanced integration tests.

@jliben jliben marked this pull request as ready for review February 22, 2022 18:20
expect(sevenFake._stage).to.eql(STAGE_HEADERS)
})

it.only('should handle $spawnOptions', function () {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops I forgot to remove the only flag here :)

@q2s2t q2s2t merged commit a607b2b into q2s2t:master Oct 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants