Skip to content

Commit

Permalink
diff: make sure to differentiate No Change vs Content change only
Browse files Browse the repository at this point in the history
This commit makes sure to have a different behavior during a diff
between a No Change at all result, vs a No Content Change result.

Before this commit we would only display a “No diff found” message in
the GH action logs.

With this new commit, if there's a content change detected we now
publish a comment containing a link to be able to preview the change.
  • Loading branch information
paulRbr committed Jun 24, 2024
1 parent ce67b2e commit 686c11d
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 10 deletions.
4 changes: 2 additions & 2 deletions src/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ function extractBumpDigest(docDigest: string, body: string): string | undefined
return (body.match(bumpDiffRegexp(docDigest)) || []).pop();
}

function shaDigest(texts: string[]): string {
function shaDigest(texts: (string | undefined)[]): string {
const hash = crypto.createHash('sha1');

texts.forEach((text) => hash.update(text, 'utf8'));
texts.forEach((text) => text && hash.update(text, 'utf8'));

return hash.digest('hex');
}
Expand Down
16 changes: 12 additions & 4 deletions src/diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { DiffResponse } from 'bump-cli';
import { bumpDiffComment, shaDigest } from './common';

export async function run(diff: DiffResponse, repo: Repo): Promise<void> {
const digestContent = [diff.markdown!];
const digestContent = [diff.markdown];
if (diff.public_url) {
digestContent.push(diff.public_url);
}
Expand All @@ -16,18 +16,26 @@ export async function run(diff: DiffResponse, repo: Repo): Promise<void> {
function buildCommentBody(docDigest: string, diff: DiffResponse, digest: string) {
const emptySpace = '';
const poweredByBump = '> _Powered by [Bump.sh](https://bump.sh)_';
const text = diff.markdown || 'No structural change, nothing to display.';

return [title(diff)]
.concat([emptySpace, diff.markdown!])
.concat([emptySpace, text])
.concat([viewDiffLink(diff), poweredByBump, bumpDiffComment(docDigest, digest)])
.join('\n');
}

function title(diff: DiffResponse): string {
const commentTitle = '🤖 API change detected:';
const structureTitle = '🤖 API structural change detected:';
const contentTitle = 'ℹ️ API content change detected:';
const breakingTitle = '🚨 Breaking API change detected:';

return diff.breaking ? breakingTitle : commentTitle;
if (diff.breaking) {
return breakingTitle;
} else if (diff.markdown) {
return structureTitle;
} else {
return contentTitle;
}
}

function viewDiffLink(diff: DiffResponse): string {
Expand Down
4 changes: 2 additions & 2 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ async function run(): Promise<void> {
await new bump.Diff(config)
.run(file1, file2, doc, hub, branch, token, 'markdown', expires)
.then((result: bump.DiffResponse | undefined) => {
if (result && 'markdown' in result) {
if (result) {
diff.run(result, repo).catch(handleErrors);
} else {
core.info('No diff found, nothing more to do.');
core.info('No changes detected, nothing more to do.');
repo.deleteExistingComment();
}

Expand Down
30 changes: 28 additions & 2 deletions tests/diff.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,39 @@ test('test github diff run process', async () => {
await diff.run(result, repo);

expect(mockedInternalRepo.prototype.createOrUpdateComment).toHaveBeenCalledWith(
`🤖 API change detected:
`🤖 API structural change detected:
* one
* two
* three
[Preview documentation](https://bump.sh/doc/my-doc/changes/654)
> _Powered by [Bump.sh](https://bump.sh)_
<!-- Bump.sh digest=${digest} doc=undefined -->`,
digest,
);
});

test('test github diff with no structural change', async () => {
const result: bump.DiffResponse = {
id: '123abc',
public_url: 'https://bump.sh/doc/my-doc/changes/654',
breaking: false,
};
const digest = '3999a0fe6ad27841bd6342128f7028ab2cea1c57';

expect(mockedInternalRepo).not.toHaveBeenCalled();

const repo = new Repo('');
await diff.run(result, repo);

expect(mockedInternalRepo.prototype.createOrUpdateComment).toHaveBeenCalledWith(
`ℹ️ API content change detected:
No structural change, nothing to display.
[Preview documentation](https://bump.sh/doc/my-doc/changes/654)
> _Powered by [Bump.sh](https://bump.sh)_
Expand Down Expand Up @@ -91,7 +117,7 @@ test('test github diff without public url', async () => {
await diff.run(result, repo);

expect(mockedInternalRepo.prototype.createOrUpdateComment).toHaveBeenCalledWith(
`🤖 API change detected:
`🤖 API structural change detected:
* one
* two
Expand Down
35 changes: 35 additions & 0 deletions tests/main.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,3 +367,38 @@ test('test action run diff with internal exception', async () => {
);
expect(mockedInternalDiff.run).toHaveBeenCalledWith(diffExample, expect.any(Repo));
});

test('test action run diff with no change', async () => {
const spyInfo = jest.spyOn(core, 'info');

mockedDiff.prototype.run.mockResolvedValue(undefined);
expect(mockedDiff.prototype.run).not.toHaveBeenCalled();
expect(mockedInternalDiff.run).not.toHaveBeenCalled();
mockedInternalRepo.prototype.getBaseFile.mockResolvedValue('my-base-file-to-diff.yml');

const restore = mockEnv({
INPUT_FILE: 'my-file-to-diff.yml',
INPUT_COMMAND: 'diff',
INPUT_FAIL_ON_BREAKING: 'true',
});

await main();

restore();

expect(spyInfo).toHaveBeenCalledWith(
expect.stringMatching('No changes detected, nothing more to do.'),
);

expect(mockedDiff.prototype.run).toHaveBeenCalledWith(
'my-base-file-to-diff.yml',
'my-file-to-diff.yml',
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
'markdown',
expect.anything(),
);
expect(mockedInternalDiff.run).not.toHaveBeenCalled();
});

0 comments on commit 686c11d

Please sign in to comment.