From d473db0899e318d4c0009c675a9297d84d212848 Mon Sep 17 00:00:00 2001 From: daz Date: Fri, 2 Aug 2024 13:46:33 -0600 Subject: [PATCH 1/2] Add tests for wrapper-validation with insufficient wrappers - Test min-wrapper-count greater than wrappers - Test caswrapper-validation with insufficient wrappers --- .../integ-test-wrapper-validation.yml | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/.github/workflows/integ-test-wrapper-validation.yml b/.github/workflows/integ-test-wrapper-validation.yml index 25d29f5e..22a5aea1 100644 --- a/.github/workflows/integ-test-wrapper-validation.yml +++ b/.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 From ac3aebda9366152f36e8b32e05ecf218b697ccc2 Mon Sep 17 00:00:00 2001 From: daz Date: Fri, 2 Aug 2024 14:40:16 -0600 Subject: [PATCH 2/2] Improve error messages for min-wrapper-count - Specific message when no wrappers are found - Better message when wrapper count is less than configured Fixes #284 --- .../src/actions/wrapper-validation/main.ts | 12 +++++++-- sources/src/wrapper-validation/validate.ts | 13 +-------- .../wrapper-validation/wrapper-validator.ts | 5 ++-- .../jest/wrapper-validation/validate.test.ts | 27 +++---------------- 4 files changed, 16 insertions(+), 41 deletions(-) diff --git a/sources/src/actions/wrapper-validation/main.ts b/sources/src/actions/wrapper-validation/main.ts index fad3e5d4..d3bf831c 100644 --- a/sources/src/actions/wrapper-validation/main.ts +++ b/sources/src/actions/wrapper-validation/main.ts @@ -18,15 +18,23 @@ export async function run(): Promise { 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('|')}`) diff --git a/sources/src/wrapper-validation/validate.ts b/sources/src/wrapper-validation/validate.ts index e8314620..85543f41 100644 --- a/sources/src/wrapper-validation/validate.ts +++ b/sources/src/wrapper-validation/validate.ts @@ -5,7 +5,6 @@ import {resolve} from 'path' export async function findInvalidWrapperJars( gitRepoRoot: string, - minWrapperCount: number, allowSnapshots: boolean, allowedChecksums: string[], previouslyValidatedChecksums: string[] = [], @@ -13,11 +12,6 @@ export async function findInvalidWrapperJars( ): Promise { 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,7 +48,6 @@ export class ValidationResult { valid: WrapperJar[] invalid: WrapperJar[] fetchedChecksums = false - errors: string[] = [] constructor(valid: WrapperJar[], invalid: WrapperJar[]) { this.valid = valid @@ -62,7 +55,7 @@ export class ValidationResult { } 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)}` diff --git a/sources/src/wrapper-validation/wrapper-validator.ts b/sources/src/wrapper-validation/wrapper-validator.ts index 8958fab3..72b20f0e 100644 --- a/sources/src/wrapper-validation/wrapper-validator.ts +++ b/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()}` ) } diff --git a/sources/test/jest/wrapper-validation/validate.test.ts b/sources/test/jest/wrapper-validation/validate.test.ts index c4b86ce5..0639b092 100644 --- a/sources/test/jest/wrapper-validation/validate.test.ts +++ b/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})