From d3ee21941a78c16695efe9b8e634a8f2921acf47 Mon Sep 17 00:00:00 2001 From: Damien Guard Date: Thu, 28 Apr 2016 12:23:44 -0700 Subject: [PATCH 1/3] Fix some specs on Windows and honor options.shell --- spec/buffered-process-spec.coffee | 23 +++++++++-------------- src/buffered-process.coffee | 2 +- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/spec/buffered-process-spec.coffee b/spec/buffered-process-spec.coffee index 643a2d411..ec01b40d7 100644 --- a/spec/buffered-process-spec.coffee +++ b/spec/buffered-process-spec.coffee @@ -16,9 +16,9 @@ describe "BufferedProcess", -> describe "when an error event is emitted by the process", -> it "calls the error handler and does not throw an exception", -> process = new BufferedProcess - command: 'bad-command-nope' + command: 'bad-command-nope1' args: ['nothing'] - options: {} + options: {shell: false} errorSpy = jasmine.createSpy().andCallFake (error) -> error.handle() process.onWillThrowError(errorSpy) @@ -28,7 +28,7 @@ describe "BufferedProcess", -> runs -> expect(window.onerror).not.toHaveBeenCalled() expect(errorSpy).toHaveBeenCalled() - expect(errorSpy.mostRecentCall.args[0].error.message).toContain 'spawn bad-command-nope ENOENT' + expect(errorSpy.mostRecentCall.args[0].error.message).toContain 'spawn bad-command-nope1 ENOENT' describe "when an error is thrown spawning the process", -> it "calls the error handler and does not throw an exception", -> @@ -53,17 +53,17 @@ describe "BufferedProcess", -> expect(errorSpy.mostRecentCall.args[0].error.message).toContain 'Something is really wrong' describe "when there is not an error handler specified", -> - it "calls the error handler and does not throw an exception", -> + it "does throw an exception", -> process = new BufferedProcess - command: 'bad-command-nope' + command: 'bad-command-nope2' args: ['nothing'] - options: {} + options: {shell: false} waitsFor -> window.onerror.callCount > 0 runs -> expect(window.onerror).toHaveBeenCalled() - expect(window.onerror.mostRecentCall.args[0]).toContain 'Failed to spawn command `bad-command-nope`' + expect(window.onerror.mostRecentCall.args[0]).toContain 'Failed to spawn command `bad-command-nope2`' expect(window.onerror.mostRecentCall.args[4].name).toBe 'BufferedProcessError' describe "on Windows", -> @@ -72,13 +72,7 @@ describe "BufferedProcess", -> beforeEach -> # Prevent any commands from actually running and affecting the host originalSpawn = ChildProcess.spawn - spyOn(ChildProcess, 'spawn').andCallFake -> - # Just spawn something that won't actually modify the host - if originalPlatform is 'win32' - originalSpawn('dir') - else - originalSpawn('ls') - + spyOn(ChildProcess, 'spawn') originalPlatform = process.platform Object.defineProperty process, 'platform', value: 'win32' @@ -117,6 +111,7 @@ describe "BufferedProcess", -> expect(stdout).toEqual '' it "calls the specified stdout callback only with whole lines", -> + # WINSPEC: Fails because command is too long, /bin/echo is *nix only, odd newlining and quoting exitCallback = jasmine.createSpy('exit callback') baseContent = "There are dozens of us! Dozens! It's as Ann as the nose on Plain's face. Can you believe that the only reason the club is going under is because it's in a terrifying neighborhood? She calls it a Mayonegg. Waiting for the Emmys. BTW did you know won 6 Emmys and was still canceled early by Fox? COME ON. I'll buy you a hundred George Michaels that you can teach to drive! Never once touched my per diem. I'd go to Craft Service, get some raw veggies, bacon, Cup-A-Soup…baby, I got a stew goin'" content = (baseContent for _ in [1..200]).join('\n') diff --git a/src/buffered-process.coffee b/src/buffered-process.coffee index 59f2a7e9a..07fcfb664 100644 --- a/src/buffered-process.coffee +++ b/src/buffered-process.coffee @@ -50,7 +50,7 @@ class BufferedProcess options ?= {} @command = command # Related to joyent/node#2318 - if process.platform is 'win32' + if process.platform is 'win32' and not options.shell? # Quote all arguments and escapes inner quotes if args? cmdArgs = args.filter (arg) -> arg? From 7542a401fff99d6ca93e7b1254bd8fc4d3672559 Mon Sep 17 00:00:00 2001 From: Damien Guard Date: Mon, 2 May 2016 15:37:36 -0700 Subject: [PATCH 2/3] BufferedProcess whole-string spec now cross-platform --- spec/buffered-process-spec.coffee | 29 +++++++++++++---------------- spec/fixtures/lorem.txt | 3 +++ spec/fixtures/shebang | 2 +- 3 files changed, 17 insertions(+), 17 deletions(-) create mode 100644 spec/fixtures/lorem.txt diff --git a/spec/buffered-process-spec.coffee b/spec/buffered-process-spec.coffee index ec01b40d7..40b96e3e1 100644 --- a/spec/buffered-process-spec.coffee +++ b/spec/buffered-process-spec.coffee @@ -1,5 +1,7 @@ ChildProcess = require 'child_process' path = require 'path' +fs = require 'fs-plus' +platform = require './spec-helper-platform' BufferedProcess = require '../src/buffered-process' describe "BufferedProcess", -> @@ -110,30 +112,25 @@ describe "BufferedProcess", -> expect(stderr).toContain 'apm - Atom Package Manager' expect(stdout).toEqual '' - it "calls the specified stdout callback only with whole lines", -> - # WINSPEC: Fails because command is too long, /bin/echo is *nix only, odd newlining and quoting + it "calls the specified stdout callback with whole lines", -> exitCallback = jasmine.createSpy('exit callback') - baseContent = "There are dozens of us! Dozens! It's as Ann as the nose on Plain's face. Can you believe that the only reason the club is going under is because it's in a terrifying neighborhood? She calls it a Mayonegg. Waiting for the Emmys. BTW did you know won 6 Emmys and was still canceled early by Fox? COME ON. I'll buy you a hundred George Michaels that you can teach to drive! Never once touched my per diem. I'd go to Craft Service, get some raw veggies, bacon, Cup-A-Soup…baby, I got a stew goin'" - content = (baseContent for _ in [1..200]).join('\n') + loremPath = require.resolve("./fixtures/lorem.txt") + content = fs.readFileSync(loremPath).toString() + baseContent = content.split('\n') stdout = '' - endLength = 10 - outputAlwaysEndsWithStew = true + allLinesEndWithNewline = true process = new BufferedProcess - command: '/bin/echo' - args: [content] + command: if platform.isWindows() then 'type' else 'cat' + args: [loremPath] options: {} stdout: (lines) -> + endsWithNewline = (lines.charAt lines.length - 1) is '\n' + if not endsWithNewline then allLinesEndWithNewline = false stdout += lines - - end = baseContent.substr(baseContent.length - endLength, endLength) - lineEndsWithStew = lines.substr(lines.length - endLength, endLength) is end - expect(lineEndsWithStew).toBeTrue - - outputAlwaysEndsWithStew = outputAlwaysEndsWithStew and lineEndsWithStew exit: exitCallback waitsFor -> exitCallback.callCount is 1 runs -> - expect(outputAlwaysEndsWithStew).toBeTrue - expect(stdout).toBe content += '\n' + expect(allLinesEndWithNewline).toBeTrue + expect(stdout).toBe content diff --git a/spec/fixtures/lorem.txt b/spec/fixtures/lorem.txt new file mode 100644 index 000000000..be8db8ab8 --- /dev/null +++ b/spec/fixtures/lorem.txt @@ -0,0 +1,3 @@ +Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur ultricies nulla id nibh aliquam, vitae euismod ipsum scelerisque. Vestibulum vulputate facilisis nisi, eu rhoncus turpis pretium ut. Curabitur facilisis urna in diam efficitur, vel maximus tellus consectetur. Suspendisse pulvinar felis sed metus tristique, a posuere dui suscipit. Ut vehicula, tellus ac blandit consequat, libero dui hendrerit elit, non pretium metus odio sed dolor. Vivamus quis volutpat ipsum. In convallis magna nec nunc tristique malesuada. Sed sed hendrerit lacus. Etiam arcu dui, consequat vel neque vitae, iaculis egestas justo. Donec lacinia odio nulla, condimentum porta erat accumsan at. Nunc vulputate nulla vel nunc fermentum egestas. +Duis ultricies libero elit, nec facilisis mi rhoncus ornare. Aliquam aliquet libero vitae arcu porttitor mattis. Vestibulum ultricies consectetur arcu, non gravida magna eleifend vel. Phasellus varius mattis ultricies. Vestibulum placerat lacus non consectetur fringilla. Duis congue, arcu iaculis vehicula hendrerit, purus odio faucibus ipsum, et fermentum massa tellus euismod nulla. Vivamus pellentesque blandit massa, sit amet hendrerit turpis congue eu. Suspendisse diam dui, vestibulum nec semper varius, maximus eu nunc. Vivamus facilisis pulvinar viverra. Praesent luctus lectus id est porttitor volutpat. Suspendisse est augue, mattis a tincidunt id, condimentum in turpis. Curabitur at erat commodo orci interdum tincidunt. Sed sodales elit odio, a placerat ipsum luctus nec. Sed maximus, justo ut pharetra pellentesque, orci mi faucibus enim, quis viverra arcu dui sed nisl. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Praesent quis velit libero. +Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia Curae; Phasellus a rutrum tortor. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia Curae; Fusce bibendum odio et neque vestibulum rutrum. Vestibulum commodo, nibh non sodales lobortis, dui ex consectetur leo, a finibus libero lectus ac diam. Etiam dui nunc, bibendum a tempor vel, vestibulum lacinia neque. Mauris consectetur odio sit amet maximus pretium. Sed rutrum nunc at ante ullamcorper fermentum. Proin at quam a mauris pellentesque viverra. Nunc pretium pulvinar ipsum. Vestibulum eu nibh ut ex gravida tempus. Praesent ut elit ut ligula tristique dapibus ut sit amet leo. Proin non molestie erat. diff --git a/spec/fixtures/shebang b/spec/fixtures/shebang index f15429b13..f343f6833 100644 --- a/spec/fixtures/shebang +++ b/spec/fixtures/shebang @@ -1,3 +1,3 @@ #!/usr/bin/ruby -puts "America – fuck yeah!" \ No newline at end of file +puts "Atom fixture test" From 48703864c87c231eb1eb3675254f7a44a9c57d9c Mon Sep 17 00:00:00 2001 From: Damien Guard Date: Tue, 3 May 2016 15:22:13 -0700 Subject: [PATCH 3/3] Switch to process.platform, stop clobbering process var and move windows block --- spec/buffered-process-spec.coffee | 69 +++++++++++++++---------------- 1 file changed, 34 insertions(+), 35 deletions(-) diff --git a/spec/buffered-process-spec.coffee b/spec/buffered-process-spec.coffee index 40b96e3e1..84d8b0440 100644 --- a/spec/buffered-process-spec.coffee +++ b/spec/buffered-process-spec.coffee @@ -1,7 +1,6 @@ ChildProcess = require 'child_process' path = require 'path' fs = require 'fs-plus' -platform = require './spec-helper-platform' BufferedProcess = require '../src/buffered-process' describe "BufferedProcess", -> @@ -17,13 +16,13 @@ describe "BufferedProcess", -> describe "when there is an error handler specified", -> describe "when an error event is emitted by the process", -> it "calls the error handler and does not throw an exception", -> - process = new BufferedProcess + bufferedProcess = new BufferedProcess command: 'bad-command-nope1' args: ['nothing'] options: {shell: false} errorSpy = jasmine.createSpy().andCallFake (error) -> error.handle() - process.onWillThrowError(errorSpy) + bufferedProcess.onWillThrowError(errorSpy) waitsFor -> errorSpy.callCount > 0 @@ -39,13 +38,13 @@ describe "BufferedProcess", -> error.code = 'EAGAIN' throw error - process = new BufferedProcess + bufferedProcess = new BufferedProcess command: 'ls' args: [] options: {} errorSpy = jasmine.createSpy().andCallFake (error) -> error.handle() - process.onWillThrowError(errorSpy) + bufferedProcess.onWillThrowError(errorSpy) waitsFor -> errorSpy.callCount > 0 @@ -56,7 +55,7 @@ describe "BufferedProcess", -> describe "when there is not an error handler specified", -> it "does throw an exception", -> - process = new BufferedProcess + new BufferedProcess command: 'bad-command-nope2' args: ['nothing'] options: {shell: false} @@ -68,37 +67,11 @@ describe "BufferedProcess", -> expect(window.onerror.mostRecentCall.args[0]).toContain 'Failed to spawn command `bad-command-nope2`' expect(window.onerror.mostRecentCall.args[4].name).toBe 'BufferedProcessError' - describe "on Windows", -> - originalPlatform = null - - beforeEach -> - # Prevent any commands from actually running and affecting the host - originalSpawn = ChildProcess.spawn - spyOn(ChildProcess, 'spawn') - originalPlatform = process.platform - Object.defineProperty process, 'platform', value: 'win32' - - afterEach -> - Object.defineProperty process, 'platform', value: originalPlatform - - describe "when the explorer command is spawned on Windows", -> - it "doesn't quote arguments of the form /root,C...", -> - new BufferedProcess({command: 'explorer.exe', args: ['/root,C:\\foo']}) - expect(ChildProcess.spawn.argsForCall[0][1][3]).toBe '"explorer.exe /root,C:\\foo"' - - it "spawns the command using a cmd.exe wrapper", -> - new BufferedProcess({command: 'dir'}) - expect(path.basename(ChildProcess.spawn.argsForCall[0][0])).toBe 'cmd.exe' - expect(ChildProcess.spawn.argsForCall[0][1][0]).toBe '/s' - expect(ChildProcess.spawn.argsForCall[0][1][1]).toBe '/d' - expect(ChildProcess.spawn.argsForCall[0][1][2]).toBe '/c' - expect(ChildProcess.spawn.argsForCall[0][1][3]).toBe '"dir"' - it "calls the specified stdout, stderr, and exit callbacks", -> stdout = '' stderr = '' exitCallback = jasmine.createSpy('exit callback') - process = new BufferedProcess + new BufferedProcess command: atom.packages.getApmPath() args: ['-h'] options: {} @@ -119,8 +92,8 @@ describe "BufferedProcess", -> baseContent = content.split('\n') stdout = '' allLinesEndWithNewline = true - process = new BufferedProcess - command: if platform.isWindows() then 'type' else 'cat' + new BufferedProcess + command: if process.platform is 'win32' then 'type' else 'cat' args: [loremPath] options: {} stdout: (lines) -> @@ -134,3 +107,29 @@ describe "BufferedProcess", -> runs -> expect(allLinesEndWithNewline).toBeTrue expect(stdout).toBe content + + describe "on Windows", -> + originalPlatform = null + + beforeEach -> + # Prevent any commands from actually running and affecting the host + originalSpawn = ChildProcess.spawn + spyOn(ChildProcess, 'spawn') + originalPlatform = process.platform + Object.defineProperty process, 'platform', value: 'win32' + + afterEach -> + Object.defineProperty process, 'platform', value: originalPlatform + + describe "when the explorer command is spawned on Windows", -> + it "doesn't quote arguments of the form /root,C...", -> + new BufferedProcess({command: 'explorer.exe', args: ['/root,C:\\foo']}) + expect(ChildProcess.spawn.argsForCall[0][1][3]).toBe '"explorer.exe /root,C:\\foo"' + + it "spawns the command using a cmd.exe wrapper when options.shell is undefined", -> + new BufferedProcess({command: 'dir'}) + expect(path.basename(ChildProcess.spawn.argsForCall[0][0])).toBe 'cmd.exe' + expect(ChildProcess.spawn.argsForCall[0][1][0]).toBe '/s' + expect(ChildProcess.spawn.argsForCall[0][1][1]).toBe '/d' + expect(ChildProcess.spawn.argsForCall[0][1][2]).toBe '/c' + expect(ChildProcess.spawn.argsForCall[0][1][3]).toBe '"dir"'