feat: update to 0.17.3, fix bugs, improve logging and error handling

README.md

@@ -42,7 +42,7 @@ Problems it can help with:
          - name: Run integration tests
            uses: data-catering/insta-integration@v3
            with:
-             data_caterer_version: 0.16.1
+             data_caterer_version: 0.17.3
    ```
 
 1. Create YAML file `insta-integration.yaml` to define your integration tests
@@ -266,7 +266,7 @@ can be found below.
 | configuration_file   | File path to configuration file                                                              | `insta-integration.yaml`                 |
 | insta_infra_folder   | Folder path to insta-infra ([this repository](https://github.com/data-catering/insta-infra)) | `${HOME}/.insta-integration/insta-infra` |
 | base_folder          | Folder path to use for execution files                                                       | `${HOME}/.insta-integration`             |
-| data_caterer_version | Version of data-caterer Docker image                                                         | `0.16.1`                                 |
+| data_caterer_version | Version of data-caterer Docker image                                                         | `0.17.3`                                 |
 
 To use these configurations, alter your
 `.github/workflows/integration-test.yaml`.
@@ -288,7 +288,7 @@ jobs:
           configuration_file: my/custom/folder/insta-integration.yaml
           insta_infra_folder: insta-infra/folder
           base_folder: execution/folder
-          data_caterer_version: 0.16.1
+          data_caterer_version: 0.17.3
 ```
 
 #### Output

__tests__/insta-integration.test.js

@@ -5,7 +5,8 @@ const {
   extractRelationships,
   extractServiceNamesAndEnv,
   shutdownApplication,
-  runApplication
+  runApplication,
+  isRunGenerationFirst
 } = require('../src/insta-integration')
 const logger = require('../src/util/log')
 const { expect } = require('@jest/globals')
@@ -185,7 +186,7 @@ describe('extractServiceFromGeneration', () => {
         generationTaskToServiceMapping
       )
     }).toThrow(
-      'Relationship defined without corresponding generation task, relationship=nonExistentTask'
+      'Relationship defined without corresponding generation task: nonExistentTask'
     )
   })
 
@@ -233,7 +234,7 @@ describe('extractServiceFromGeneration', () => {
         generationTaskToServiceMapping
       )
     }).toThrow(
-      'Relationship defined without corresponding generation task, relationship=undefined'
+      'Relationship defined without corresponding generation task: undefined'
     )
   })
 })
@@ -373,7 +374,7 @@ describe('extractRelationships', () => {
         currentPlan
       )
     }).toThrow(
-      'Relationship should follow pattern: <generation name>||<fields> or <data source>||<generation name>||<fields>, relationship=c'
+      'Invalid relationship format (expected: <name>||<fields> or <source>||<name>||<fields>): child.field'
     )
   })
 
@@ -732,8 +733,13 @@ describe('runApplication', () => {
       true
     )
     expect(result).not.toBeNull()
-    expect(logger.info).toHaveBeenCalledWith('Running application/job')
-    expect(logger.info).toHaveBeenCalledWith('Application 1 exited with code 0')
+    expect(logger.info).toHaveBeenCalledWith(
+      '[App] Running application (index: 1)'
+    )
+    expect(logger.logSuccess).toHaveBeenCalledWith(
+      '[App]',
+      'Application 1 completed (exit code: 0)'
+    )
   })
 
   it('runs the application without waiting for it to finish', async () => {
@@ -752,7 +758,31 @@ describe('runApplication', () => {
       false
     )
     expect(result).not.toBeNull()
-    expect(logger.info).toHaveBeenCalledWith('Running application/job')
+    expect(logger.info).toHaveBeenCalledWith(
+      '[App] Running application (index: 1)'
+    )
+  })
+
+  it('returns runApp object that can be used with shutdownApplication', async () => {
+    const runConf = {
+      command: 'sleep 10',
+      commandWaitForFinish: false
+    }
+    const configFolder = '/tmp/insta-integration-test/config'
+    const baseFolder = '/tmp/insta-integration-test/base'
+    const appIndex = 1
+    const result = await runApplication(
+      runConf,
+      configFolder,
+      baseFolder,
+      appIndex,
+      false
+    )
+    expect(result).not.toBeNull()
+    expect(result.runApp).toBeDefined()
+    expect(typeof result.runApp.kill).toBe('function')
+    // Clean up - kill the sleep process
+    result.runApp.kill()
   })
 
   it('throws an error if the command fails', async () => {
@@ -763,10 +793,10 @@ describe('runApplication', () => {
     await expect(
       runApplication(runConf, configFolder, baseFolder, appIndex, true)
     ).rejects.toThrow('Application 1 exited with code 127')
-    expect(logger.error).toHaveBeenCalledWith({
-      command: 'invalid_command',
-      message: 'Failed to run application/job'
-    })
+    expect(logger.logError).toHaveBeenCalledWith(
+      '[App]',
+      'Application 1 exited with code 127'
+    )
   })
 
   it('returns null if no command is defined', async () => {
@@ -782,7 +812,39 @@ describe('runApplication', () => {
       true
     )
     expect(result).toBeNull()
-    expect(logger.debug).toHaveBeenCalledWith('No command defined')
+    expect(logger.debug).toHaveBeenCalledWith('[App] No command defined')
+  })
+})
+
+describe('isRunGenerationFirst', () => {
+  it('returns true when generateFirst is undefined', () => {
+    const runConf = { test: {} }
+    expect(isRunGenerationFirst(runConf)).toBe(true)
+  })
+
+  it('returns true when generateFirst is boolean true with test defined', () => {
+    const runConf = { generateFirst: true, test: {} }
+    expect(isRunGenerationFirst(runConf)).toBe(true)
+  })
+
+  it('returns true when generateFirst is string "true" with test defined', () => {
+    const runConf = { generateFirst: 'true', test: {} }
+    expect(isRunGenerationFirst(runConf)).toBe(true)
+  })
+
+  it('returns false when generateFirst is boolean false', () => {
+    const runConf = { generateFirst: false, test: {} }
+    expect(isRunGenerationFirst(runConf)).toBe(false)
+  })
+
+  it('returns false when generateFirst is string "false"', () => {
+    const runConf = { generateFirst: 'false', test: {} }
+    expect(isRunGenerationFirst(runConf)).toBe(false)
+  })
+
+  it('returns false when generateFirst is true but test is not defined', () => {
+    const runConf = { generateFirst: true }
+    expect(isRunGenerationFirst(runConf)).toBe(false)
   })
 })
 
@@ -796,20 +858,21 @@ describe('shutdownApplication', () => {
     const applicationProcess = { runApp: { kill: jest.fn() } }
     shutdownApplication(applicationProcess)
     expect(applicationProcess.runApp.kill).toHaveBeenCalled()
-    expect(logger.debug).toHaveBeenCalledWith('Killing application now')
+    expect(logger.debug).toHaveBeenCalledWith('[App] Application terminated')
   })
 
   it('does not attempt to shut down if the application is already stopped', () => {
     const applicationProcess = { runApp: null }
     shutdownApplication(applicationProcess)
-    expect(logger.debug).toHaveBeenCalledWith('Application already stopped')
+    expect(logger.debug).toHaveBeenCalledWith(
+      '[App] Application already stopped'
+    )
   })
 
   it('does not attempt to shut down if the application process is null', () => {
     const applicationProcess = null
     shutdownApplication(applicationProcess)
-    expect(logger.debug).toHaveBeenCalledWith(
-      'Application process is null, not attempting to shutdown'
-    )
+    // When application process is null, shutdownApplication does nothing (no logs)
+    expect(logger.debug).not.toHaveBeenCalled()
   })
 })

__tests__/main.test.js

@@ -28,6 +28,18 @@ describe('getBaseFolder', () => {
       'Base folder configuration is not defined'
     )
   })
+
+  it('uses actions input when default base folder is empty', () => {
+    core.getInput.mockReturnValueOnce('/from-actions-input')
+    expect(getBaseFolder('')).toBe('/from-actions-input')
+  })
+
+  it('throws an error when both input and default are empty', () => {
+    core.getInput.mockReturnValueOnce('')
+    expect(() => getBaseFolder('')).toThrow(
+      'Base folder configuration is not defined'
+    )
+  })
 })
 
 describe('getDataCatererVersion', () => {
@@ -36,7 +48,7 @@ describe('getDataCatererVersion', () => {
   })
 
   it('returns the default data caterer version if not provided', () => {
-    expect(getDataCatererVersion('')).toBe('0.16.1')
+    expect(getDataCatererVersion('')).toBe('0.17.3')
   })
 })
 

__tests__/util/docker.test.js

@@ -27,12 +27,10 @@ describe('runDockerImage', () => {
     expect(() => runDockerImage('docker run my-image', 1)).toThrow(
       'command failed'
     )
-    expect(logger.error).toHaveBeenCalledWith(
-      'Failed to run data caterer docker image'
-    )
-    expect(logger.error).toHaveBeenCalledWith(
-      'Failed to retrieve container logs, container-name=',
-      'data-caterer-1'
+    expect(logger.logError).toHaveBeenCalledWith(
+      '[Docker]',
+      'Failed to run data-caterer container',
+      expect.any(Error)
     )
     expect(core.setFailed).toHaveBeenCalledWith(new Error('command failed'))
   })
@@ -103,4 +101,13 @@ describe('waitForContainerToFinish', () => {
       'docker ps -q -f name=my-container -f status=exited'
     )
   })
+
+  it('should reject when isContainerFinished throws an error', async () => {
+    execSync.mockImplementation(() => {
+      throw new Error('Docker command failed')
+    })
+    await expect(waitForContainerToFinish('my-container')).rejects.toThrow(
+      'Docker command failed'
+    )
+  })
 })

__tests__/util/file.test.js

@@ -75,7 +75,7 @@ describe('writeToFile', () => {
     jest.spyOn(fs, 'mkdirSync')
     jest.spyOn(fs, 'writeFileSync')
     jest.spyOn(logger, 'debug')
-    jest.spyOn(logger, 'error')
+    jest.spyOn(logger, 'logError')
     jest.spyOn(yaml, 'dump')
   })
 
@@ -115,7 +115,7 @@ describe('writeToFile', () => {
   it('should log debug messages', () => {
     writeToFile('folder', 'test.txt', 'content', true)
     expect(logger.debug).toHaveBeenCalledWith(
-      'Creating file, file-path=folder/test.txt'
+      '[Config] Writing file: folder/test.txt'
     )
   })
 
@@ -126,8 +126,10 @@ describe('writeToFile', () => {
     expect(() => writeToFile('folder', 'error.txt', 'content', true)).toThrow(
       'Write error'
     )
-    expect(logger.error).toHaveBeenCalledWith(
-      'Failed to write to file, file-name=folder/error.txt'
+    expect(logger.logError).toHaveBeenCalledWith(
+      '[Config]',
+      'Failed to write file: folder/error.txt',
+      expect.any(Error)
     )
   })
 })
@@ -156,7 +158,9 @@ describe('cleanAppDoneFiles', () => {
       throw new Error('unlink failed')
     })
     await cleanAppDoneFiles(parsedConfig, sharedFolder, 10)
-    expect(logger.debug).toHaveBeenCalledWith(new Error('unlink failed'))
+    expect(logger.debug).toHaveBeenCalledWith(
+      '[Config] No file to clean: app-0-done'
+    )
   })
 })
 
@@ -200,9 +204,7 @@ describe('checkFileExistsWithTimeout', () => {
     })
     await expect(
       checkFileExistsWithTimeout('/path/to/file', 0, 10)
-    ).rejects.toThrow(
-      'File did not exist and was not created during the timeout, file=/path/to/file'
-    )
+    ).rejects.toThrow('Timeout: File not created within 10ms: /path/to/file')
   })
 })
 
@@ -218,11 +220,14 @@ describe('showLogFileContent', () => {
     const logContent = 'line1\nline2\nline3'
     fs.existsSync.mockReturnValue(true)
     fs.readFileSync.mockReturnValue(logContent)
+    logger.isDebugEnabled = jest.fn().mockReturnValue(true)
     showLogFileContent('/path/to/log')
-    expect(logger.debug).toHaveBeenCalledWith('Showing application logs')
-    expect(logger.debug).toHaveBeenCalledWith('line1')
-    expect(logger.debug).toHaveBeenCalledWith('line2')
-    expect(logger.debug).toHaveBeenCalledWith('line3')
+    expect(logger.debug).toHaveBeenCalledWith(
+      '[App] Application logs from: /path/to/log'
+    )
+    expect(logger.debug).toHaveBeenCalledWith('  line1')
+    expect(logger.debug).toHaveBeenCalledWith('  line2')
+    expect(logger.debug).toHaveBeenCalledWith('  line3')
   })
 })
 

__tests__/util/insta-infra.test.js

@@ -74,9 +74,7 @@ describe('runServices', () => {
         ['service1', 'unsupportedService'],
         {}
       )
-    ).toThrow(
-      'Found unsupported insta-infra service in configuration, service=unsupportedService'
-    )
+    ).toThrow('Unsupported service: unsupportedService')
   })
 
   it('should log error and check container status if running services fail', () => {
@@ -88,8 +86,29 @@ describe('runServices', () => {
     expect(() => runServices('/path/to/insta-infra', ['service1'], {})).toThrow(
       'run failed'
     )
-    expect(logger.error).toHaveBeenCalledWith(
-      expect.stringContaining('Failed to run services=service1')
+    expect(logger.logError).toHaveBeenCalledWith(
+      '[Service]',
+      'Failed to start services: service1'
     )
   })
+
+  it('should check all services when running services fail', () => {
+    execSync.mockReturnValueOnce('service1 service2 service3')
+    execSync.mockImplementationOnce(() => {
+      throw new Error('run failed')
+    })
+    isContainerFinished.mockReturnValue(false)
+    expect(() =>
+      runServices(
+        '/path/to/insta-infra',
+        ['service1', 'service2', 'service3'],
+        {}
+      )
+    ).toThrow('run failed')
+    // All three services should be checked
+    expect(isContainerFinished).toHaveBeenCalledTimes(3)
+    expect(isContainerFinished).toHaveBeenCalledWith('service1')
+    expect(isContainerFinished).toHaveBeenCalledWith('service2')
+    expect(isContainerFinished).toHaveBeenCalledWith('service3')
+  })
 })

action.yml

@@ -18,7 +18,7 @@ inputs:
     default: '/home/runner/work'
   data_caterer_version:
     description: 'Version of data-caterer Docker image'
-    default: '0.16.1'
+    default: '0.17.3'
 
 # Define your outputs here.
 outputs: