Improve error messages for min-wrapper-count (#321)

Author: bigdaz

.github/workflows/integ-test-wrapper-validation.yml

@@ -54,6 +54,7 @@ jobs:
       with:
         # to allow the invalid wrapper jar present in test data
         allow-checksums: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
+        min-wrapper-count: 10
 
     - name: Check outcome
       env:
@@ -98,3 +99,62 @@ jobs:
           echo "'outputs.failed-wrapper' has unexpected content: $FAILED_WRAPPERS"
           exit 1
         fi
+
+  test-validation-minimum-wrapper-count:
+    runs-on: ubuntu-latest
+    steps:
+    - name: Checkout sources
+      uses: actions/checkout@v4
+    - name: Initialize integ-test
+      uses: ./.github/actions/init-integ-test
+
+    - name: Run wrapper-validation-action
+      id: action-test
+      uses: ./wrapper-validation
+      with:
+        # to allow the invalid wrapper jar present in test data
+        allow-checksums: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
+        min-wrapper-count: 11
+      # Expected to fail; validated below
+      continue-on-error: true
+
+    - name: Check outcome
+      env:
+        # Evaluate workflow expressions here as env variable values instead of inside shell script
+        # below to not accidentally inject code into shell script or break its syntax
+        VALIDATION_FAILED: ${{ steps.action-test.outcome == 'failure' }}
+      run: |
+        if [ "$VALIDATION_FAILED" != "true" ] ; then
+          echo "Expected validation to fail, but it didn't"
+          exit 1
+        fi
+
+  test-validation-zero-wrappers:
+    runs-on: ubuntu-latest
+    steps:
+    - name: Checkout sources
+      uses: actions/checkout@v4 # Checkout the repository with no wrappers
+      with:
+        sparse-checkout: |
+          .github/actions
+          dist
+          wrapper-validation
+    - name: Initialize integ-test
+      uses: ./.github/actions/init-integ-test
+
+    - name: Run wrapper-validation-action
+      id: action-test
+      uses: ./wrapper-validation
+      # Expected to fail; validated below
+      continue-on-error: true
+
+    - name: Check outcome
+      env:
+        # Evaluate workflow expressions here as env variable values instead of inside shell script
+        # below to not accidentally inject code into shell script or break its syntax
+        VALIDATION_FAILED: ${{ steps.action-test.outcome == 'failure' }}
+      run: |
+        if [ "$VALIDATION_FAILED" != "true" ] ; then
+          echo "Expected validation to fail, but it didn't"
+          exit 1
+        fi

sources/src/actions/wrapper-validation/main.ts

@@ -18,15 +18,23 @@ export async function run(): Promise<void> {
 
         const result = await validate.findInvalidWrapperJars(
             path.resolve('.'),
-            +core.getInput('min-wrapper-count'),
             core.getInput('allow-snapshots') === 'true',
             core.getInput('allow-checksums').split(',')
         )
         if (result.isValid()) {
             core.info(result.toDisplayString())
+
+            const minWrapperCount = +core.getInput('min-wrapper-count')
+            if (result.valid.length < minWrapperCount) {
+                const message =
+                    result.valid.length === 0
+                        ? 'Wrapper validation failed: no Gradle Wrapper jars found. Did you forget to checkout the repository?'
+                        : `Wrapper validation failed: expected at least ${minWrapperCount} Gradle Wrapper jars, but found ${result.valid.length}.`
+                core.setFailed(message)
+            }
         } else {
             core.setFailed(
-                `Gradle Wrapper Validation Failed!\n  See https://github.com/gradle/actions/blob/main/docs/wrapper-validation.md#reporting-failures\n${result.toDisplayString()}`
+                `At least one Gradle Wrapper Jar failed validation!\n  See https://github.com/gradle/actions/blob/main/docs/wrapper-validation.md#validation-failures\n${result.toDisplayString()}`
             )
             if (result.invalid.length > 0) {
                 core.setOutput('failed-wrapper', `${result.invalid.map(w => w.path).join('|')}`)

sources/src/wrapper-validation/validate.ts

@@ -5,19 +5,13 @@ import {resolve} from 'path'
 
 export async function findInvalidWrapperJars(
     gitRepoRoot: string,
-    minWrapperCount: number,
     allowSnapshots: boolean,
     allowedChecksums: string[],
     previouslyValidatedChecksums: string[] = [],
     knownValidChecksums: checksums.WrapperChecksums = checksums.KNOWN_CHECKSUMS
 ): Promise<ValidationResult> {
     const wrapperJars = await find.findWrapperJars(gitRepoRoot)
     const result = new ValidationResult([], [])
-    if (wrapperJars.length < minWrapperCount) {
-        result.errors.push(
-            `Expected to find at least ${minWrapperCount} Gradle Wrapper JARs but got only ${wrapperJars.length}`
-        )
-    }
     if (wrapperJars.length > 0) {
         const notYetValidatedWrappers = []
         for (const wrapperJar of wrapperJars) {
@@ -54,15 +48,14 @@ export class ValidationResult {
     valid: WrapperJar[]
     invalid: WrapperJar[]
     fetchedChecksums = false
-    errors: string[] = []
 
     constructor(valid: WrapperJar[], invalid: WrapperJar[]) {
         this.valid = valid
         this.invalid = invalid
     }
 
     isValid(): boolean {
-        return this.invalid.length === 0 && this.errors.length === 0
+        return this.invalid.length === 0
     }
 
     toDisplayString(): string {
@@ -72,10 +65,6 @@ export class ValidationResult {
                 this.invalid
             )}`
         }
-        if (this.errors.length > 0) {
-            if (displayString.length > 0) displayString += '\n'
-            displayString += `✗ Other validation errors:\n  ${this.errors.join(`\n  `)}`
-        }
         if (this.valid.length > 0) {
             if (displayString.length > 0) displayString += '\n'
             displayString += `✓ Found known Gradle Wrapper JAR files:\n${ValidationResult.toDisplayList(this.valid)}`

sources/src/wrapper-validation/wrapper-validator.ts

@@ -20,20 +20,19 @@ export async function validateWrappers(
 
     const result = await findInvalidWrapperJars(
         workspaceRoot,
-        0,
         config.allowSnapshotWrappers(),
         allowedChecksums,
         previouslyValidatedChecksums
     )
     if (result.isValid()) {
         await core.group('All Gradle Wrapper jars are valid', async () => {
-            core.info(`Loaded previously validated checksums from cache: ${previouslyValidatedChecksums.join(', ')}`)
+            core.debug(`Loaded previously validated checksums from cache: ${previouslyValidatedChecksums.join(', ')}`)
             core.info(result.toDisplayString())
         })
     } else {
         core.info(result.toDisplayString())
         throw new JobFailure(
-            `Gradle Wrapper Validation Failed!\n  See https://github.com/gradle/actions/blob/main/docs/wrapper-validation.md#validation-failures\n${result.toDisplayString()}`
+            `At least one Gradle Wrapper Jar failed validation!\n  See https://github.com/gradle/actions/blob/main/docs/wrapper-validation.md#validation-failures\n${result.toDisplayString()}`
         )
     }
 

sources/test/jest/wrapper-validation/validate.test.ts

@@ -12,7 +12,7 @@ const baseDir = path.resolve('./test/jest/wrapper-validation')
 const tmpDir = path.resolve('./test/jest/tmp')
 
 test('succeeds if all found wrapper jars are valid', async () => {
-  const result = await validate.findInvalidWrapperJars(baseDir, 3, false, [
+  const result = await validate.findInvalidWrapperJars(baseDir, false, [
     'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'
   ])
 
@@ -29,7 +29,7 @@ test('succeeds if all found wrapper jars are valid', async () => {
 })
 
 test('succeeds if all found wrapper jars are previously valid', async () => {
-  const result = await validate.findInvalidWrapperJars(baseDir, 3, false, [], [
+  const result = await validate.findInvalidWrapperJars(baseDir, false, [], [
     'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855',
     '3888c76faa032ea8394b8a54e04ce2227ab1f4be64f65d450f8509fe112d38ce'
   ])
@@ -50,7 +50,6 @@ test('succeeds if all found wrapper jars are valid (and checksums are fetched fr
   const knownValidChecksums = new WrapperChecksums()
   const result = await validate.findInvalidWrapperJars(
     baseDir,
-    1,
     false,
     ['e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'],
     [],
@@ -71,7 +70,7 @@ test('succeeds if all found wrapper jars are valid (and checksums are fetched fr
 })
 
 test('fails if invalid wrapper jars are found', async () => {
-  const result = await validate.findInvalidWrapperJars(baseDir, 3, false, [])
+  const result = await validate.findInvalidWrapperJars(baseDir, false, [])
 
   expect(result.isValid()).toBe(false)
 
@@ -102,26 +101,6 @@ test('fails if invalid wrapper jars are found', async () => {
   )
 })
 
-test('fails if not enough wrapper jars are found', async () => {
-  const result = await validate.findInvalidWrapperJars(baseDir, 4, false, [])
-
-  expect(result.isValid()).toBe(false)
-
-  expect(result.errors).toEqual([
-    'Expected to find at least 4 Gradle Wrapper JARs but got only 3'
-  ])
-
-  expect(result.toDisplayString()).toBe(
-    '✗ Found unknown Gradle Wrapper JAR files:\n' +
-      '  e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 data/invalid/gradle-wrapper.jar\n' +
-      '  e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 data/invalid/gradlе-wrapper.jar\n' + // homoglyph
-      '✗ Other validation errors:\n' +
-      '  Expected to find at least 4 Gradle Wrapper JARs but got only 3\n' +
-      '✓ Found known Gradle Wrapper JAR files:\n' +
-      '  3888c76faa032ea8394b8a54e04ce2227ab1f4be64f65d450f8509fe112d38ce data/valid/gradle-wrapper.jar'
-  )
-})
-
 test('can save and load checksums', async () => {
   const cacheDir = path.join(tmpDir, 'wrapper-validation-cache')
   fs.rmSync(cacheDir, {recursive: true, force: true})