Skip to content

Commit 1f706ab

Browse files
sethkfmanCal-L
andauthored
chore: fitness quality gate to only allow TS & TSX files in app directory (#9723)
## **Description** This PR adds a fitness function to fail if a .js or .jsx file is added to the app/ directory. ## **Related issues** Fixes: https://github.com/orgs/MetaMask/projects/60/views/3?pane=issue&itemId=59597337 ## **Manual testing steps** 1. Attempt to push a .js or .jsx file in the app directory 2. See the fitness function fail 3. ## **Screenshots/Recordings** NA See unit tests ### **Before** <!-- [screenshots/recordings] --> ### **After** - Commit [outside of app directory](https://github.com/MetaMask/metamask-mobile/actions/runs/9353695806/job/25744808362) - Commit [inside of app directory](https://github.com/MetaMask/metamask-mobile/actions/runs/9455916396/job/26046720818?pr=9723) ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Cal Leung <cleun007@gmail.com>
1 parent c3d5e71 commit 1f706ab

File tree

5 files changed

+102
-2
lines changed

5 files changed

+102
-2
lines changed

.github/scripts/fitness-functions/common/constants.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,7 @@ enum AUTOMATION_TYPE {
77
PRE_PUSH_HOOK = 'pre-push-hook',
88
}
99

10-
export { EXCLUDE_REGEX, AUTOMATION_TYPE };
10+
// only allow TS and TSX files in the app directory only
11+
const APP_FOLDER_JS_REGEX = /^(app).*\.(js|jsx)$/;
12+
13+
export { EXCLUDE_REGEX, APP_FOLDER_JS_REGEX, AUTOMATION_TYPE };

.github/scripts/fitness-functions/common/shared.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,34 @@ function filterDiffByFilePath(diff: string, regex: RegExp): string {
3333
return filteredDiff;
3434
}
3535

36+
function restrictedFilePresent(diff: string, regex: RegExp): boolean {
37+
// split by `diff --git` and remove the first element which is empty
38+
const diffBlocks = diff.split(`diff --git`).slice(1);
39+
let jsOrJsxFilePresent = false;
40+
diffBlocks
41+
.map((block) => block.trim())
42+
.filter((block) => {
43+
block
44+
// get the first line of the block which has the paths
45+
.split('\n')[0]
46+
.trim()
47+
// split the two paths
48+
.split(' ')
49+
// remove `a/` and `b/` from the paths
50+
.map((path) => path.substring(2))
51+
// if at least one of the two paths matches the regex, filter the
52+
// corresponding diff block in
53+
.forEach((path) => {
54+
if (regex.test(path)) {
55+
// Not excluded, include in check
56+
jsOrJsxFilePresent = true;
57+
}
58+
});
59+
return jsOrJsxFilePresent;
60+
})
61+
return jsOrJsxFilePresent;
62+
}
63+
3664
// This function returns all lines that are additions to files that are being
3765
// modified but that previously already existed. Example:
3866
// diff --git a/test.js b/test.js
@@ -111,6 +139,7 @@ function hasNumberOfCodeBlocksIncreased(
111139

112140
export {
113141
filterDiffByFilePath,
142+
restrictedFilePresent,
114143
filterDiffFileCreations,
115144
filterDiffLineAdditions,
116145
hasNumberOfCodeBlocksIncreased,

.github/scripts/fitness-functions/rules/index.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { preventJavaScriptFileAdditions } from './javascript-additions';
12
import { preventCodeBlocksRule } from './prevent-code-blocks';
23

34
const RULES: IRule[] = [
@@ -6,6 +7,11 @@ const RULES: IRule[] = [
67
fn: preventCodeBlocksRule,
78
docURL: '[WIP] No documentation exists for this rule yet.',
89
},
10+
{
11+
name: 'Check for js or jsx file being added',
12+
fn: preventJavaScriptFileAdditions,
13+
docURL: '[WIP] No documentation exists for this rule yet.',
14+
},
915
];
1016

1117
interface IRule {
@@ -32,5 +38,5 @@ function runFitnessFunctionRule(rule: IRule, diff: string): void {
3238
}
3339
}
3440

35-
export { RULES, runFitnessFunctionRule };
41+
export { RULES, runFitnessFunctionRule, preventJavaScriptFileAdditions };
3642
export type { IRule };
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import {
2+
generateModifyFilesDiff,
3+
generateCreateFileDiff,
4+
} from '../common/test-data';
5+
import { preventJavaScriptFileAdditions } from './javascript-additions';
6+
7+
describe('preventJavaScriptFileAdditions()', (): void => {
8+
it('should pass when receiving an empty diff', (): void => {
9+
const testDiff = '';
10+
11+
const hasRulePassed = preventJavaScriptFileAdditions(testDiff);
12+
13+
expect(hasRulePassed).toBe(true);
14+
});
15+
16+
it('should pass when receiving a diff with a new TS file on the shared folder', (): void => {
17+
const testDiff = [
18+
generateModifyFilesDiff('new-file.ts', 'foo', 'bar'),
19+
generateModifyFilesDiff('old-file.js', undefined, 'pong'),
20+
generateCreateFileDiff('app/test.ts', 'yada yada yada yada'),
21+
].join('');
22+
23+
const hasRulePassed = preventJavaScriptFileAdditions(testDiff);
24+
25+
expect(hasRulePassed).toBe(true);
26+
});
27+
28+
it('should not pass when receiving a diff with a new JS file on the shared folder', (): void => {
29+
const testDiff = [
30+
generateModifyFilesDiff('new-file.ts', 'foo', 'bar'),
31+
generateModifyFilesDiff('old-file.js', undefined, 'pong'),
32+
generateCreateFileDiff('app/test.js', 'yada yada yada yada'),
33+
].join('');
34+
35+
const hasRulePassed = preventJavaScriptFileAdditions(testDiff);
36+
37+
expect(hasRulePassed).toBe(false);
38+
});
39+
40+
it('should not pass when receiving a diff with a new JSX file on the shared folder', (): void => {
41+
const testDiff = [
42+
generateModifyFilesDiff('new-file.ts', 'foo', 'bar'),
43+
generateModifyFilesDiff('old-file.js', undefined, 'pong'),
44+
generateCreateFileDiff('app/test.jsx', 'yada yada yada yada'),
45+
].join('');
46+
47+
const hasRulePassed = preventJavaScriptFileAdditions(testDiff);
48+
49+
expect(hasRulePassed).toBe(false);
50+
});
51+
});
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import { APP_FOLDER_JS_REGEX } from '../common/constants';
2+
import { restrictedFilePresent } from '../common/shared';
3+
4+
function preventJavaScriptFileAdditions(diff: string): boolean {
5+
if (restrictedFilePresent(diff, APP_FOLDER_JS_REGEX)) {
6+
return false;
7+
}
8+
return true;
9+
}
10+
11+
export { preventJavaScriptFileAdditions };

0 commit comments

Comments
 (0)