Fix child_process test in node <16 (#4059)

DataDog · Feb 14, 2024 · a3d44af · a3d44af
1 parent a406a24
commit a3d44af
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 42 deletions.
diff --git a/packages/datadog-instrumentations/test/child_process.spec.js b/packages/datadog-instrumentations/test/child_process.spec.js
@@ -3,6 +3,7 @@
 const { promisify } = require('util')
 const agent = require('../../dd-trace/test/plugins/agent')
 const dc = require('dc-polyfill')
+const { NODE_MAJOR } = require('../../../version')
 
 describe('child process', () => {
   const modules = ['child_process', 'node:child_process']
@@ -303,18 +304,37 @@ describe('child process', () => {
               it('should execute error callback with `exit 1` command', () => {
                 let childError
                 try {
-                  childProcess[methodName]('node -e "process.exit(1)"', { shell: true })
+                  childProcess[methodName]('node -e "process.exit(1)"')
                 } catch (error) {
                   childError = error
                 } finally {
                   expect(start).to.have.been.calledOnceWith({
                     command: 'node -e "process.exit(1)"',
-                    shell: true,
+                    shell: false,
                     error: childError
                   })
                   expect(finish).to.have.been.calledOnce
                 }
               })
+              if (methodName !== 'execFileSync' || NODE_MAJOR > 16) {
+                // when a process return an invalid code, in node <=16, in execFileSync with shell:true
+                // an exception is not thrown
+                it('should execute error callback with `exit 1` command with shell: true', () => {
+                  let childError
+                  try {
+                    childProcess[methodName]('node -e "process.exit(1)"', { shell: true })
+                  } catch (error) {
+                    childError = error
+                  } finally {
+                    expect(start).to.have.been.calledOnceWith({
+                      command: 'node -e "process.exit(1)"',
+                      shell: true,
+                      error: childError
+                    })
+                    expect(finish).to.have.been.calledOnce
+                  }
+                })
+              }
             })
           })
         })

diff --git a/packages/datadog-plugin-child_process/test/index.spec.js b/packages/datadog-plugin-child_process/test/index.spec.js
@@ -4,6 +4,7 @@ const ChildProcessPlugin = require('../src')
 const { storage } = require('../../datadog-core')
 const agent = require('../../dd-trace/test/plugins/agent')
 const { expectSomeSpan } = require('../../dd-trace/test/plugins/helpers')
+const { NODE_MAJOR } = require('../../../version')
 
 function noop () {}
 
@@ -519,53 +520,58 @@ describe('Child process plugin', () => {
                 }
               })
 
-              it('should be instrumented with error code (override shell default behavior)', (done) => {
-                const command = [ 'node', '-badOption' ]
-                const options = {
-                  stdio: 'pipe',
-                  shell: true
-                }
-                const errorExpected = {
-                  type: 'system',
-                  name: 'command_execution',
-                  error: 1,
-                  meta: {
-                    component: 'subprocess',
-                    'cmd.shell': 'node -badOption',
-                    'cmd.exit_code': '9'
+              if (methodName !== 'execFileSync' || NODE_MAJOR > 16) {
+                // when a process return an invalid code, in node <=16, in execFileSync with shell:true
+                // an exception is not thrown
+                it('should be instrumented with error code (override shell default behavior)', (done) => {
+                  const command = [ 'node', '-badOption' ]
+                  const options = {
+                    stdio: 'pipe',
+                    shell: true
                   }
-                }
 
-                const noErrorExpected = {
-                  type: 'system',
-                  name: 'command_execution',
-                  error: 0,
-                  meta: {
-                    component: 'subprocess',
-                    'cmd.shell': 'node -badOption',
-                    'cmd.exit_code': '9'
+                  const errorExpected = {
+                    type: 'system',
+                    name: 'command_execution',
+                    error: 1,
+                    meta: {
+                      component: 'subprocess',
+                      'cmd.shell': 'node -badOption',
+                      'cmd.exit_code': '9'
+                    }
                   }
-                }
 
-                const args = normalizeArgs(methodName, command, options)
+                  const noErrorExpected = {
+                    type: 'system',
+                    name: 'command_execution',
+                    error: 0,
+                    meta: {
+                      component: 'subprocess',
+                      'cmd.shell': 'node -badOption',
+                      'cmd.exit_code': '9'
+                    }
+                  }
 
-                if (async) {
-                  expectSomeSpan(agent, errorExpected).then(done, done)
-                  const res = childProcess[methodName].apply(null, args)
-                  res.on('close', noop)
-                } else {
-                  try {
-                    if (methodName === 'spawnSync') {
-                      expectSomeSpan(agent, noErrorExpected).then(done, done)
-                    } else {
-                      expectSomeSpan(agent, errorExpected).then(done, done)
+                  const args = normalizeArgs(methodName, command, options)
+
+                  if (async) {
+                    expectSomeSpan(agent, errorExpected).then(done, done)
+                    const res = childProcess[methodName].apply(null, args)
+                    res.on('close', noop)
+                  } else {
+                    try {
+                      if (methodName === 'spawnSync') {
+                        expectSomeSpan(agent, noErrorExpected).then(done, done)
+                      } else {
+                        expectSomeSpan(agent, errorExpected).then(done, done)
+                      }
+                      childProcess[methodName].apply(null, args)
+                    } catch {
+                      // process exit with code 1, exceptions are expected
                     }
-                    childProcess[methodName].apply(null, args)
-                  } catch {
-                    // process exit with code 1, exceptions are expected
                   }
-                }
-              })
+                })
+              }
             })
           })
         })