From ec4ebdccdbaf8a0d1c7a075b7759a055b1469d89 Mon Sep 17 00:00:00 2001 From: Ethan Dennis Date: Tue, 16 Jun 2020 12:15:15 -0700 Subject: [PATCH 1/4] Initial globbing tests --- .vscode/launch.json | 17 ++++++++++++ __tests__/services/comments.test.ts | 4 +-- __tests__/services/state.test.ts | 40 +++++++++++++++++++++++++++++ src/services/state.ts | 8 ++++-- 4 files changed, 64 insertions(+), 5 deletions(-) create mode 100644 .vscode/launch.json create mode 100644 __tests__/services/state.test.ts diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 0000000..bce7992 --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,17 @@ +{ + // Use IntelliSense to learn about possible attributes. + // Hover to view descriptions of existing attributes. + // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 + "version": "0.2.0", + "configurations": [ + { + "name": "Debug Jest Tests", + "type": "node", + "request": "launch", + "program": "${workspaceFolder}/node_modules/.bin/jest", + "args": ["--runInBand"], + "console": "integratedTerminal", + "internalConsoleOptions": "neverOpen" + } + ] +} diff --git a/__tests__/services/comments.test.ts b/__tests__/services/comments.test.ts index 09ea308..36d66d6 100644 --- a/__tests__/services/comments.test.ts +++ b/__tests__/services/comments.test.ts @@ -1,8 +1,6 @@ -import * as path from 'path'; import { getCommentBody } from '../../src/services'; -import { Constants } from '../../src/constants'; -test('read valid config file', () => { +test('link to md5 of filepath', () => { const markdown = 'markdown'; const files = ['package.json', 'src/services/comments.ts']; const prNumber = 42; diff --git a/__tests__/services/state.test.ts b/__tests__/services/state.test.ts new file mode 100644 index 0000000..4336cbd --- /dev/null +++ b/__tests__/services/state.test.ts @@ -0,0 +1,40 @@ +import { isCommentApplicable } from '../../src/services'; +import { Comment, Change, ChangeType } from '../../src/models'; + +test('match recursively', () => { + const comment: Comment = { + pathFilter: ['app/**'], + markdown: 'Woohoo!', + blocking: false + }; + + const changes: Change[] = [ + { + file: 'app/models/foo.rb', + changeType: ChangeType.edit + } + ]; + + const result = isCommentApplicable(comment, changes); + + expect(result).toEqual(changes.map(c => c.file)); +}); + +test('remove exclusion patterns', () => { + const comment: Comment = { + pathFilter: ['app/**', '!app/models/*.h'], + markdown: 'Everything except headers', + blocking: false + }; + + const changes: Change[] = [ + { + file: 'app/models/main.h', + changeType: ChangeType.edit + } + ]; + + const result = isCommentApplicable(comment, changes); + + expect(result).toEqual([]); +}); diff --git a/src/services/state.ts b/src/services/state.ts index 7c22aa3..c13b9bf 100644 --- a/src/services/state.ts +++ b/src/services/state.ts @@ -75,7 +75,11 @@ export async function getTargetState( }; } -function isCommentApplicable(comment: Comment, changes: Change[]): string[] { +// TODO: Rename +export function isCommentApplicable( + comment: Comment, + changes: Change[] +): string[] { const results: string[] = []; const options: IOptions = { dot: true, nocase: true }; @@ -149,7 +153,7 @@ function isCommentApplicable(comment: Comment, changes: Change[]): string[] { } // Check if any exclusion should filter out the match - for (const exclusion in exclusions) { + for (const exclusion of exclusions) { // First exclusion to match will negate the inclusion match const matcher = new Minimatch(exclusion, options); const match = matcher.match(change.file); From db73b8acedd791c71a563de41f22fb9fdfe8c7a8 Mon Sep 17 00:00:00 2001 From: Ethan Dennis Date: Tue, 16 Jun 2020 12:17:48 -0700 Subject: [PATCH 2/4] More test coverage for path modifiers --- __tests__/services/state.test.ts | 69 ++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/__tests__/services/state.test.ts b/__tests__/services/state.test.ts index 4336cbd..f8c6ea8 100644 --- a/__tests__/services/state.test.ts +++ b/__tests__/services/state.test.ts @@ -38,3 +38,72 @@ test('remove exclusion patterns', () => { expect(result).toEqual([]); }); + +test('match new files only', () => { + const comment: Comment = { + pathFilter: ['+app/**'], + markdown: 'Only new files', + blocking: false + }; + + const changes: Change[] = [ + { + file: 'app/models/old.rb', + changeType: ChangeType.edit + }, + { + file: 'app/models/new.rb', + changeType: ChangeType.add + } + ]; + + const result = isCommentApplicable(comment, changes); + + expect(result).toEqual(['app/models/new.rb']); +}); + +test('match deleted files only', () => { + const comment: Comment = { + pathFilter: ['-app/**'], + markdown: 'Only deleted files', + blocking: false + }; + + const changes: Change[] = [ + { + file: 'app/models/old.rb', + changeType: ChangeType.edit + }, + { + file: 'app/models/deleted.rb', + changeType: ChangeType.delete + } + ]; + + const result = isCommentApplicable(comment, changes); + + expect(result).toEqual(['app/models/deleted.rb']); +}); + +test('match edited files only', () => { + const comment: Comment = { + pathFilter: ['~app/**'], + markdown: 'Only deleted files', + blocking: false + }; + + const changes: Change[] = [ + { + file: 'app/models/old.rb', + changeType: ChangeType.edit + }, + { + file: 'app/models/deleted.rb', + changeType: ChangeType.delete + } + ]; + + const result = isCommentApplicable(comment, changes); + + expect(result).toEqual(['app/models/old.rb']); +}); From c017d1f8813403400998ba82397f2fb8d6a3a8ab Mon Sep 17 00:00:00 2001 From: Ethan Dennis Date: Tue, 16 Jun 2020 12:19:22 -0700 Subject: [PATCH 3/4] Rename isCommentApplicable to getMatchingFilePaths --- __tests__/services/state.test.ts | 12 ++++++------ src/services/state.ts | 5 ++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/__tests__/services/state.test.ts b/__tests__/services/state.test.ts index f8c6ea8..a10fa6f 100644 --- a/__tests__/services/state.test.ts +++ b/__tests__/services/state.test.ts @@ -1,4 +1,4 @@ -import { isCommentApplicable } from '../../src/services'; +import { getMatchingFilePaths } from '../../src/services'; import { Comment, Change, ChangeType } from '../../src/models'; test('match recursively', () => { @@ -15,7 +15,7 @@ test('match recursively', () => { } ]; - const result = isCommentApplicable(comment, changes); + const result = getMatchingFilePaths(comment, changes); expect(result).toEqual(changes.map(c => c.file)); }); @@ -34,7 +34,7 @@ test('remove exclusion patterns', () => { } ]; - const result = isCommentApplicable(comment, changes); + const result = getMatchingFilePaths(comment, changes); expect(result).toEqual([]); }); @@ -57,7 +57,7 @@ test('match new files only', () => { } ]; - const result = isCommentApplicable(comment, changes); + const result = getMatchingFilePaths(comment, changes); expect(result).toEqual(['app/models/new.rb']); }); @@ -80,7 +80,7 @@ test('match deleted files only', () => { } ]; - const result = isCommentApplicable(comment, changes); + const result = getMatchingFilePaths(comment, changes); expect(result).toEqual(['app/models/deleted.rb']); }); @@ -103,7 +103,7 @@ test('match edited files only', () => { } ]; - const result = isCommentApplicable(comment, changes); + const result = getMatchingFilePaths(comment, changes); expect(result).toEqual(['app/models/old.rb']); }); diff --git a/src/services/state.ts b/src/services/state.ts index c13b9bf..3d59ec5 100644 --- a/src/services/state.ts +++ b/src/services/state.ts @@ -33,7 +33,7 @@ export async function getTargetState( ); for (const comment of allComments) { - const matches = isCommentApplicable(comment, changes); + const matches = getMatchingFilePaths(comment, changes); const isApplicable = matches.length > 0; const commentMatchText = `${comment.markdown}${Constants.CannedTextSeparator}`; @@ -75,8 +75,7 @@ export async function getTargetState( }; } -// TODO: Rename -export function isCommentApplicable( +export function getMatchingFilePaths( comment: Comment, changes: Change[] ): string[] { From 0ec9c2c030fd91d5b140b7843ed86667599ee575 Mon Sep 17 00:00:00 2001 From: Ethan Dennis Date: Tue, 16 Jun 2020 12:59:16 -0700 Subject: [PATCH 4/4] Disallow multiple path filter modifiers --- __tests__/services/state.test.ts | 57 ++++++++++++++++++++++++++++++++ package-lock.json | 2 +- src/services/state.ts | 6 ++++ 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/__tests__/services/state.test.ts b/__tests__/services/state.test.ts index a10fa6f..dafa2cf 100644 --- a/__tests__/services/state.test.ts +++ b/__tests__/services/state.test.ts @@ -1,6 +1,25 @@ import { getMatchingFilePaths } from '../../src/services'; import { Comment, Change, ChangeType } from '../../src/models'; +test('* matches everything', () => { + const comment: Comment = { + pathFilter: ['*'], + markdown: 'Woohoo!', + blocking: false + }; + + const changes: Change[] = [ + { + file: 'app/models/foo.rb', + changeType: ChangeType.edit + } + ]; + + const result = getMatchingFilePaths(comment, changes); + + expect(result).toEqual(changes.map(c => c.file)); +}); + test('match recursively', () => { const comment: Comment = { pathFilter: ['app/**'], @@ -20,6 +39,25 @@ test('match recursively', () => { expect(result).toEqual(changes.map(c => c.file)); }); +test('ignore case', () => { + const comment: Comment = { + pathFilter: ['app/**'], + markdown: 'Woohoo!', + blocking: false + }; + + const changes: Change[] = [ + { + file: 'APP/MODELS/FOO.rb', + changeType: ChangeType.edit + } + ]; + + const result = getMatchingFilePaths(comment, changes); + + expect(result).toEqual(changes.map(c => c.file)); +}); + test('remove exclusion patterns', () => { const comment: Comment = { pathFilter: ['app/**', '!app/models/*.h'], @@ -107,3 +145,22 @@ test('match edited files only', () => { expect(result).toEqual(['app/models/old.rb']); }); + +test('disallow multiple modifiers', () => { + const comment: Comment = { + pathFilter: ['!~app/**'], + markdown: 'Not edited files', + blocking: false + }; + + const changes: Change[] = [ + { + file: 'app/models/old.rb', + changeType: ChangeType.edit + } + ]; + + expect(() => getMatchingFilePaths(comment, changes)).toThrow( + 'Multiple path modifiers are not supported' + ); +}); diff --git a/package-lock.json b/package-lock.json index 4421dd8..736e7d2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,5 +1,5 @@ { - "name": "typescript-action", + "name": "nitpicker", "version": "0.0.0", "lockfileVersion": 1, "requires": true, diff --git a/src/services/state.ts b/src/services/state.ts index 3d59ec5..1fddc95 100644 --- a/src/services/state.ts +++ b/src/services/state.ts @@ -12,6 +12,8 @@ import { getExistingComments } from '.'; import { IOptions, Minimatch } from 'minimatch'; import { Constants } from '../constants'; +const pathModifiers = ['!', '+', '-', '~']; + export async function getTargetState( octokit: github.GitHub, allComments: Comment[], @@ -91,6 +93,10 @@ export function getMatchingFilePaths( } else { inclusions.push(pathFilter); } + + if (pathModifiers.includes(pathFilter[1])) { + throw new Error('Multiple path modifiers are not supported'); + } } for (const change of changes) {