Merge pull request #11642 from atom/dg-buffered-process-options-shell

Fix some Buffered-Process specs, honor options.shell in Atom

spec/buffered-process-spec.coffee

@@ -1,5 +1,6 @@
 ChildProcess = require 'child_process'
 path = require 'path'
+fs = require 'fs-plus'
 BufferedProcess  = require '../src/buffered-process'
 
 describe "BufferedProcess", ->
@@ -15,20 +16,20 @@ 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
-            command: 'bad-command-nope'
+          bufferedProcess = new BufferedProcess
+            command: 'bad-command-nope1'
             args: ['nothing']
-            options: {}
+            options: {shell: false}
 
           errorSpy = jasmine.createSpy().andCallFake (error) -> error.handle()
-          process.onWillThrowError(errorSpy)
+          bufferedProcess.onWillThrowError(errorSpy)
 
           waitsFor -> errorSpy.callCount > 0
 
           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", ->
@@ -37,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
 
@@ -53,56 +54,24 @@ 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", ->
-        process = new BufferedProcess
-          command: 'bad-command-nope'
+      it "does throw an exception", ->
+        new BufferedProcess
+          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", ->
-    originalPlatform = null
-
-    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')
-
-      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: {}
@@ -116,29 +85,51 @@ describe "BufferedProcess", ->
       expect(stderr).toContain 'apm - Atom Package Manager'
       expect(stdout).toEqual ''
 
-  it "calls the specified stdout callback only with whole lines", ->
+  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
-    process = new BufferedProcess
-      command: '/bin/echo'
-      args: [content]
+    allLinesEndWithNewline = true
+    new BufferedProcess
+      command: if process.platform is 'win32' 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
+
+  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"'

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.

spec/fixtures/shebang

@@ -1,3 +1,3 @@
 #!/usr/bin/ruby
 
-puts "America – fuck yeah!"
+puts "Atom fixture test"

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?