Improve coverage and clean the execution

.github/workflows/integration-test.yml

@@ -67,9 +67,9 @@ jobs:
           test "$LANG" = "$CONFIG_LOCALE"
           test "$(cat ${ROOTFS_DIR}/etc/timezone)" = "$CONFIG_TIMEZONE"
 
-      - name: Remove test label from PR
+      - name: Remove test label from PR (if set)
         uses: actions-ecosystem/action-remove-labels@v1
-        if: always()
+        if: ${{ github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'test') }}
         with:
           labels: test
 

__test__/configure.test.ts

@@ -49,4 +49,15 @@ describe('Configure', () => {
       expect.not.stringContaining('secretpassword')
     )
   })
+
+  it('should fall back to default configuration values if no user settings present', async () => {
+    jest.spyOn(core, 'info').mockImplementation()
+    jest.spyOn(core, 'getInput').mockReturnValue('')
+    jest.spyOn(core, 'getBooleanInput').mockReturnValue(false)
+    jest.spyOn(config, 'validateConfig').mockReturnValue(Promise.resolve())
+
+    await configure()
+
+    expect(config.validateConfig).toHaveBeenCalledWith(config.DEFAULT_CONFIG)
+  })
 })

__test__/pi-gen.test.ts

@@ -15,7 +15,9 @@ jest.mock('../src/pi-gen-config', () => ({
   writeToFile: jest.fn()
 }))
 
-const mockPiGenDependencies = () => {
+const mockPiGenDependencies = (
+  stageDirectories = Object.values(PiGenStages)
+) => {
   jest
     .spyOn(fs.promises, 'stat')
     .mockResolvedValueOnce({isDirectory: () => true} as fs.Stats)
@@ -31,7 +33,7 @@ const mockPiGenDependencies = () => {
       isFile: () => true,
       isDirectory: () => false
     } as Dirent,
-    ...(Object.values(PiGenStages).map(stage => ({
+    ...(stageDirectories.map(stage => ({
       name: stage,
       isDirectory: () => true,
       isFile: () => false
@@ -43,7 +45,7 @@ const mockPiGenDependencies = () => {
 
 describe('PiGen', () => {
   it.each(['invalid-pi-gen-path', tmp.fileSync().name])(
-    'should fail on invalid pi-gen path',
+    'should fail on invalid pi-gen path = %s',
     async piGenPath => {
       await expect(
         async () => await PiGen.getInstance(piGenPath, DEFAULT_CONFIG)
@@ -69,6 +71,17 @@ describe('PiGen', () => {
     ).rejects.toThrow()
   })
 
+  it('should fail on missing required stage entries at pi-gen path', async () => {
+    mockPiGenDependencies(['stage0', 'stage1'])
+    jest
+      .spyOn(fs.promises, 'stat')
+      .mockResolvedValue({isDirectory: () => true} as fs.Stats)
+
+    await expect(
+      async () => await PiGen.getInstance('pi-gen-dir', DEFAULT_CONFIG)
+    ).rejects.toThrow()
+  })
+
   it('mounts all stage paths as Docker volumes', async () => {
     const piGenDir = 'pi-gen'
     mockPiGenDependencies()
@@ -88,10 +101,10 @@ describe('PiGen', () => {
       ['-c', `/${piGenDir}/config`],
       expect.objectContaining({
         cwd: piGenDir,
-        env: {
+        env: expect.objectContaining({
           PIGEN_DOCKER_OPTS:
-            '-v /any/stage/path:/any/stage/path -v /pi-gen/stage0:/pi-gen/stage0'
-        }
+            '-v /any/stage/path:/any/stage/path -v /pi-gen/stage0:/pi-gen/stage0 -e DEBIAN_FRONTEND=noninteractive'
+        })
       })
     )
   })
@@ -112,9 +125,10 @@ describe('PiGen', () => {
       ['-c', `/${piGenDir}/config`],
       expect.objectContaining({
         cwd: piGenDir,
-        env: {
-          PIGEN_DOCKER_OPTS: '-v /foo:/bar -v /pi-gen/stage0:/pi-gen/stage0'
-        }
+        env: expect.objectContaining({
+          PIGEN_DOCKER_OPTS:
+            '-v /foo:/bar -v /pi-gen/stage0:/pi-gen/stage0 -e DEBIAN_FRONTEND=noninteractive'
+        })
       })
     )
   })
@@ -142,10 +156,14 @@ describe('PiGen', () => {
   })
 
   it('configures NOOBS export for stages that export images', async () => {
-    let stageList = [tmp.dirSync().name, tmp.dirSync().name]
+    const piGenDir = 'pi-gen'
+    mockPiGenDependencies()
+    jest.spyOn(fs, 'realpathSync').mockReturnValueOnce('/pi-gen/stage0')
+
+    const stageList = [tmp.dirSync().name, tmp.dirSync().name]
     fs.writeFileSync(`${stageList[0]}/EXPORT_IMAGE`, '')
 
-    await PiGen.getInstance('', {
+    await PiGen.getInstance(piGenDir, {
       stageList: stageList,
       enableNoobs: 'true'
     } as PiGenConfig)
@@ -189,7 +207,11 @@ describe('PiGen', () => {
     [false, 'no stage message', 'info', 0],
     [true, 'no stage message', 'info', 1],
     [false, '[00:00:00] stage message', 'info', 1],
-    [true, 'warning message', 'warning', 1]
+    [true, 'warning message', 'warning', 1],
+    [false, '#6 [1/3] FROM docker.io', 'warning', 0],
+    [true, '#7 [2/3] RUN', 'warning', 0],
+    [false, ' #6 [1/3] FROM docker.io', 'info', 0],
+    [true, '   #7 [2/3] RUN', 'info', 1]
   ])(
     'handles log messages if verbose = %s',
     (verbose, line, stream, nCalls) => {

jest.config.js

@@ -4,7 +4,7 @@ module.exports = {
   coverageThreshold: {
     global: {
       statements: 96,
-      branches: 79,
+      branches: 92,
       functions: 96,
       lines: 97
     }

src/configure.ts

@@ -9,48 +9,51 @@ export async function configure(): Promise<PiGenConfig> {
     const userConfig = DEFAULT_CONFIG
 
     userConfig.imgName = core.getInput('image-name', {required: true})
+
+    const stageList = core.getInput('stage-list').split(/\s+/)
     userConfig.stageList =
-      core.getInput('stage-list').split(/\s+/) ?? DEFAULT_CONFIG.stageList
-    userConfig.release = core.getInput('release') ?? DEFAULT_CONFIG.release
+      stageList.length > 0 ? stageList : DEFAULT_CONFIG.stageList
+
+    userConfig.release = core.getInput('release') || DEFAULT_CONFIG.release
     userConfig.deployCompression =
-      core.getInput('compression') ?? DEFAULT_CONFIG.deployCompression
+      core.getInput('compression') || DEFAULT_CONFIG.deployCompression
     userConfig.compressionLevel =
-      core.getInput('compression-level') ?? DEFAULT_CONFIG.compressionLevel
+      core.getInput('compression-level') || DEFAULT_CONFIG.compressionLevel
     userConfig.localeDefault =
-      core.getInput('locale') ?? DEFAULT_CONFIG.localeDefault
+      core.getInput('locale') || DEFAULT_CONFIG.localeDefault
     userConfig.targetHostname =
-      core.getInput('hostname') ?? DEFAULT_CONFIG.targetHostname
+      core.getInput('hostname') || DEFAULT_CONFIG.targetHostname
     userConfig.keyboardKeymap =
-      core.getInput('keyboard-keymap') ?? DEFAULT_CONFIG.keyboardKeymap
+      core.getInput('keyboard-keymap') || DEFAULT_CONFIG.keyboardKeymap
     userConfig.keyboardLayout =
-      core.getInput('keyboard-layout') ?? DEFAULT_CONFIG.keyboardLayout
+      core.getInput('keyboard-layout') || DEFAULT_CONFIG.keyboardLayout
     userConfig.timezoneDefault =
-      core.getInput('timezone') ?? DEFAULT_CONFIG.timezoneDefault
+      core.getInput('timezone') || DEFAULT_CONFIG.timezoneDefault
     userConfig.firstUserName =
-      core.getInput('username') ?? DEFAULT_CONFIG.firstUserName
+      core.getInput('username') || DEFAULT_CONFIG.firstUserName
     userConfig.firstUserPass =
-      core.getInput('password') ?? DEFAULT_CONFIG.firstUserPass
+      core.getInput('password') || DEFAULT_CONFIG.firstUserPass
     userConfig.disableFirstBootUserRename =
-      core.getInput('disable-first-boot-user-rename') ??
+      core.getInput('disable-first-boot-user-rename') ||
       DEFAULT_CONFIG.disableFirstBootUserRename
-    userConfig.wpaEssid = core.getInput('wpa-essid') ?? DEFAULT_CONFIG.wpaEssid
+    userConfig.wpaEssid = core.getInput('wpa-essid') || DEFAULT_CONFIG.wpaEssid
     userConfig.wpaPassword =
-      core.getInput('wpa-password') ?? DEFAULT_CONFIG.wpaPassword
+      core.getInput('wpa-password') || DEFAULT_CONFIG.wpaPassword
     userConfig.wpaCountry =
-      core.getInput('wpa-country') ?? DEFAULT_CONFIG.wpaCountry
+      core.getInput('wpa-country') || DEFAULT_CONFIG.wpaCountry
     userConfig.enableSsh =
-      core.getInput('enable-ssh') ?? DEFAULT_CONFIG.enableSsh
-    userConfig.useQcow2 = core.getInput('use-qcow2') ?? DEFAULT_CONFIG.useQcow2
+      core.getInput('enable-ssh') || DEFAULT_CONFIG.enableSsh
+    userConfig.useQcow2 = core.getInput('use-qcow2') || DEFAULT_CONFIG.useQcow2
     userConfig.enableNoobs =
-      core.getBooleanInput('enable-noobs').toString() ??
+      core.getBooleanInput('enable-noobs')?.toString() ||
       DEFAULT_CONFIG.enableNoobs
     userConfig.exportLastStageOnly =
-      core.getBooleanInput('export-last-stage-only').toString() ??
+      core.getBooleanInput('export-last-stage-only')?.toString() ||
       DEFAULT_CONFIG.exportLastStageOnly
     userConfig.dockerOpts = core.getInput('docker-opts')
-    userConfig.setfcap = core.getInput('setfcap') ?? DEFAULT_CONFIG.setfcap
+    userConfig.setfcap = core.getInput('setfcap') || DEFAULT_CONFIG.setfcap
     userConfig.piGenRelease =
-      core.getInput('pi-gen-release') ?? DEFAULT_CONFIG.piGenRelease
+      core.getInput('pi-gen-release') || DEFAULT_CONFIG.piGenRelease
 
     await validateConfig(userConfig)
 

src/pi-gen-config.ts

@@ -35,7 +35,7 @@ export interface PiGenConfig {
 export const DEFAULT_CONFIG: PiGenConfig = {
   imgName: 'test',
   piGenRelease: 'Raspberry Pi reference',
-  release: 'bullseye',
+  release: 'bookworm',
   deployCompression: 'zip',
   compressionLevel: '6',
   localeDefault: 'en_GB.UTF-8',
@@ -47,7 +47,7 @@ export const DEFAULT_CONFIG: PiGenConfig = {
   enableSsh: '0',
   pubkeyOnlySsh: '0',
   stageList: ['stage*'],
-  useQcow2: '1',
+  useQcow2: '0',
   enableNoobs: 'false',
   exportLastStageOnly: 'true'
 }

src/pi-gen.ts

@@ -9,10 +9,7 @@ import * as colors from 'ansi-colors'
 
 export class PiGen {
   private configFilePath: string
-  private piGenBuildLogPattern = new RegExp(
-    '^\\s*\\[(?:\\d{2}:?){3}\\].*',
-    'gm'
-  )
+  private piGenBuildLogPattern = /^\s*\[(?:\d{2}:?){3}\]/gm
 
   constructor(
     private piGenDirectory: string,
@@ -25,7 +22,7 @@ export class PiGen {
     piGenDirectory: string,
     config: PiGenConfig
   ): Promise<PiGen> {
-    if (!PiGen.validatePigenDirectory(piGenDirectory)) {
+    if (!(await PiGen.validatePigenDirectory(piGenDirectory))) {
       throw new Error(`pi-gen directory at ${piGenDirectory} is invalid`)
     }
 
@@ -54,10 +51,14 @@ export class PiGen {
   }
 
   async build(verbose = false): Promise<exec.ExecOutput> {
-    let dockerOpts = this.getStagesAsDockerMounts()
+    // By default, we'll pass all user stages as mounts to the Docker run and we'll configure
+    // apt to not report progress (which can become excessive).
+    let dockerOpts = `${this.getStagesAsDockerMounts()} -e DEBIAN_FRONTEND=noninteractive`
+
     if (this.config.dockerOpts !== undefined && this.config.dockerOpts !== '') {
       dockerOpts = `${this.config.dockerOpts} ${dockerOpts}`
     }
+
     core.debug(
       `Running pi-gen build with PIGEN_DOCKER_OPTS="${dockerOpts}" and config: ${JSON.stringify(
         this.config
@@ -70,7 +71,8 @@ export class PiGen {
       {
         cwd: this.piGenDirectory,
         env: {
-          PIGEN_DOCKER_OPTS: dockerOpts
+          PIGEN_DOCKER_OPTS: dockerOpts,
+          DEBIAN_FRONTEND: 'noninteractive'
         },
         listeners: {
           stdline: (line: string) => this.logOutput(line, verbose, 'info'),
@@ -161,9 +163,15 @@ export class PiGen {
 
   logOutput(line: string, verbose: boolean, stream: 'info' | 'warning'): void {
     const isPiGenStatusMessage = this.piGenBuildLogPattern.test(line)
+
     if (verbose || isPiGenStatusMessage) {
-      line = isPiGenStatusMessage ? colors.bold(line) : line
-      stream === 'info' ? core.info(line) : core.warning(line)
+      line = isPiGenStatusMessage ? colors.bold(colors.unstyle(line)) : line
+
+      // Do not issue warning annotations for Docker BuildKit progress messages.
+      // No clue how to better suppress/redirect them for now.
+      stream === 'info' || line.match(/^\s*#\d+\s/)
+        ? core.info(line)
+        : core.warning(line)
     }
   }