Skip to content

Commit

Permalink
Fix hash issue with builds (#4013)
Browse files Browse the repository at this point in the history
* Fix hash issue with builds

* Update build.test.js
  • Loading branch information
marrobi authored Jun 27, 2024
1 parent 6909584 commit fc4abe1
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 14 deletions.
6 changes: 4 additions & 2 deletions .github/scripts/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// These tests can be run from the dev container using the run-tests.sh script
//
const { createHash } = require('crypto');
const { create } = require('domain');

async function getCommandFromComment({ core, context, github }) {
const commentUsername = context.payload.comment.user.login;
Expand Down Expand Up @@ -292,11 +293,12 @@ function getRefIdForBranch(branchName) {
return createShortHash(`refs/heads/${branchName}\n`);
}
function createShortHash(ref) {
const hash = createHash('sha2').update(ref, 'utf8').digest('hex')
const hash = createHash('sha512').update(ref, 'utf8').digest('hex');
return hash.substring(0, 8);
}

module.exports = {
getCommandFromComment,
labelAsExternalIfAuthorDoesNotHaveWriteAccess
labelAsExternalIfAuthorDoesNotHaveWriteAccess,
createShortHash
}
32 changes: 20 additions & 12 deletions .github/scripts/build.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { getCommandFromComment, labelAsExternalIfAuthorDoesNotHaveWriteAccess } = require('./build.js')
const { getCommandFromComment, labelAsExternalIfAuthorDoesNotHaveWriteAccess, createShortHash } = require('./build.js')
const { createGitHubContext, PR_NUMBER, outputFor, toHaveComment } = require('./test-helpers.js')

expect.extend({
Expand Down Expand Up @@ -146,7 +146,7 @@ describe('getCommandFromComment', () => {
owner: 'someOwner',
repo: 'someRepo',
issue_number: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES,
bodyMatcher: /Running tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `cbce50da`\)/,
bodyMatcher: /Running tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `291ae84f`\)/,
});
});
});
Expand Down Expand Up @@ -326,7 +326,7 @@ describe('getCommandFromComment', () => {
owner: 'someOwner',
repo: 'someRepo',
issue_number: PR_NUMBER.FORK_NON_DOCS_CHANGES,
bodyMatcher: /Running tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `6db070b1`\)/,
bodyMatcher: /Running tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `607c7437`\)/,
});
});
})
Expand Down Expand Up @@ -355,7 +355,7 @@ describe('getCommandFromComment', () => {
owner: 'someOwner',
repo: 'someRepo',
issue_number: PR_NUMBER.FORK_NON_DOCS_CHANGES,
bodyMatcher: /Running tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `6db070b1`\)/,
bodyMatcher: /Running tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `607c7437`\)/,
});
});
})
Expand All @@ -381,7 +381,7 @@ describe('getCommandFromComment', () => {
owner: 'someOwner',
repo: 'someRepo',
issue_number: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES,
bodyMatcher: /Running extended tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `cbce50da`\)/,
bodyMatcher: /Running extended tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `291ae84f`\)/,
});
});
});
Expand All @@ -407,7 +407,7 @@ describe('getCommandFromComment', () => {
owner: 'someOwner',
repo: 'someRepo',
issue_number: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES,
bodyMatcher: /Running extended AAD tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `cbce50da`\)/,
bodyMatcher: /Running extended AAD tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `291ae84f`\)/,
});
});
});
Expand All @@ -433,7 +433,7 @@ describe('getCommandFromComment', () => {
owner: 'someOwner',
repo: 'someRepo',
issue_number: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES,
bodyMatcher: /Running shared service tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `cbce50da`\)/,
bodyMatcher: /Running shared service tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `291ae84f`\)/,
});
});
});
Expand Down Expand Up @@ -549,7 +549,7 @@ describe('getCommandFromComment', () => {
owner: 'someOwner',
repo: 'someRepo',
issue_number: PR_NUMBER.FORK_NON_DOCS_CHANGES,
bodyMatcher: /Running extended tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `6db070b1`\)/,
bodyMatcher: /Running extended tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `607c7437`\)/,
});
});
})
Expand Down Expand Up @@ -679,13 +679,13 @@ goes here`,
});

test('should set prRefId output', async () => {
// Using a PR number of 123 should give a refid of 'cbce50da'
// Using a PR number of 123 should give a refid of '291ae84f'
// Based on running `echo "refs/pull/123/merge" | shasum | cut -c1-8` (as per the original bash scripts)
const context = createCommentContext({
pullRequestNumber: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES
});
await getCommandFromComment({ core, context, github });
expect(outputFor(mockCoreSetOutput, 'prRefId')).toBe('cbce50da');
expect(outputFor(mockCoreSetOutput, 'prRefId')).toBe('291ae84f');
});

test('should not set branchRefId output for PR from forked repo', async () => {
Expand All @@ -701,13 +701,13 @@ goes here`,

test('should set branchRefId for PR from upstream repo', async () => {
// Using PR 123 which is faked as a PR from the upstream repo
// The Using a PR number of 123 should give a refid of '71f7c907'
// The Using a PR number of 123 should give a refid of '6b751c8f'
// Based on running `echo "refs/heads/pr-head-ref" | shasum | cut -c1-8` (as per the original bash scripts)
const context = createCommentContext({
pullRequestNumber: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES
});
await getCommandFromComment({ core, context, github });
expect(outputFor(mockCoreSetOutput, 'branchRefId')).toBe('71f7c907');
expect(outputFor(mockCoreSetOutput, 'branchRefId')).toBe('6b751c8f');
});

test('should set prHeadSha output', async () => {
Expand Down Expand Up @@ -770,4 +770,12 @@ goes here`,
});
});

describe('createShortHash creates a short hash from a long hash', () => {
test('should return the first 8 characters of the hash', () => {
const longHash = '0123456789abcdef';
const shortHash = '1c043fbe';
expect(createShortHash(longHash)).toBe(shortHash);
}
);
});
});

0 comments on commit fc4abe1

Please sign in to comment.