Meta: Move feature checker to Vitest (refined-github#7145)

Author: fregante

.github/workflows/features.yml

@@ -19,7 +19,7 @@ jobs:
           node-version-file: package.json
           cache: npm
       - run: npm ci
-      - run: npm run test:features
+      - run: npm run vitest -- --testNamePattern features.test
 
   TestURLs:
     runs-on: ubuntu-latest
@@ -34,7 +34,7 @@ jobs:
         id: changed-files
         uses: tj-actions/changed-files@v35
         with:
-          files: source/features/*.tsx
+          files: source/features/*.{tsx,css}
 
       - name: Every new/edited file must have a test URL
         run: bash build/verify-test-urls.sh ${{steps.changed-files.outputs.all_changed_files}}

.github/workflows/test.yml

@@ -25,7 +25,7 @@ jobs:
           node-version-file: package.json
           cache: npm
       - run: npm ci
-      - run: npm run vitest
+      - run: npm run vitest -- --testNamePattern !features.test
 
   Lint:
     runs-on: ubuntu-latest

.xo-config.json

@@ -14,6 +14,7 @@
 		"no-alert": "off",
 		"no-warning-comments": "off",
 		"unicorn/no-nested-ternary": "off",
+		"unicorn/better-regex": "off",
 		"unicorn/prefer-top-level-await": "off",
 		"unicorn/prefer-dom-node-dataset": "off",
 		"unicorn/expiring-todo-comments": ["warn", {

build/__snapshots__/features-meta.json

@@ -0,0 +1,186 @@
+import {test, describe, assert} from 'vitest';
+import {parse, join} from 'node:path';
+import {existsSync, readdirSync, readFileSync} from 'node:fs';
+import regexJoin from 'regex-join';
+import fastIgnore from 'fast-ignore';
+
+import {isFeaturePrivate} from '../source/helpers/feature-utils.js';
+import {getImportedFeatures, getFeaturesMeta} from './readme-parser.js';
+
+const isGitIgnored = fastIgnore(readFileSync('.gitignore', 'utf8'));
+
+const noScreenshotExceptions = new Set([
+	// Only add feature here if it's a shortcut only and/or extremely clear by name or description
+	'sort-conversations-by-update-time',
+	'prevent-pr-merge-panel-opening',
+	'infinite-scroll',
+	'command-palette-navigation-shortcuts',
+	'comment-fields-keyboard-shortcuts',
+	'copy-on-y',
+	'create-release-shortcut',
+	'pagination-hotkey',
+	'profile-hotkey',
+	'repo-wide-file-finder',
+	'select-all-notifications-shortcut',
+	'selection-in-new-tab',
+	'submission-via-ctrl-enter-everywhere',
+
+	'hide-navigation-hover-highlight', // TODO: Add side-by-side gif
+	'hide-inactive-deployments', // TODO: side-by-side png
+	'esc-to-deselect-line', // TODO Add gif with key overlay
+	'hide-newsfeed-noise', // TODO: Add side-by-side png
+	'scrollable-areas', // TODO: Add side-by-side png
+]);
+
+const entryPoint = 'source/refined-github.ts';
+const entryPointSource = readFileSync(entryPoint);
+const importedFeatures = getImportedFeatures();
+const featuresInReadme = getFeaturesMeta();
+
+// We used to enforce the filetype, but this is no longer possible with new URLs
+// https://github.com/refined-github/refined-github/pull/7130
+const imageRegex = /\.(png|gif)$/;
+
+const rghUploadsRegex = /refined-github[/]refined-github[/]assets[/]/;
+
+const screenshotRegex = regexJoin(imageRegex, /|/, rghUploadsRegex);
+
+class FeatureFile {
+	readonly id: FeatureID;
+	readonly path: string;
+	constructor(readonly name: string) {
+		this.id = parse(name).name as FeatureID;
+		this.path = join('source/features', name);
+	}
+
+	exists(): boolean {
+		return existsSync(this.path);
+	}
+
+	// eslint-disable-next-line n/prefer-global/buffer -- Type only
+	contents(): Buffer {
+		return readFileSync(this.path);
+	}
+
+	get tsx(): FeatureFile {
+		if (this.name.endsWith('.gql')) {
+			const id = importedFeatures.find(featureId => this.id.startsWith(featureId));
+			if (id) {
+				return new FeatureFile(id + '.tsx');
+			}
+		}
+
+		return new FeatureFile(this.id + '.tsx');
+	}
+
+	get css(): FeatureFile {
+		return new FeatureFile(this.id + '.css');
+	}
+}
+
+function validateCss(file: FeatureFile): void {
+	const isImportedByEntrypoint = entryPointSource.includes(`import './features/${file.name}';`);
+	if (!file.tsx.exists()) {
+		assert(
+			isImportedByEntrypoint,
+			`Should be imported by \`${entryPoint}\` or removed if it is not needed`,
+		);
+		return;
+	}
+
+	assert(
+		file.tsx.contents().includes(`import './${file.name}';`),
+		`Should be imported by \`${file.tsx.name}\``,
+	);
+
+	assert(
+		!isImportedByEntrypoint,
+		`Should only be imported by \`${file.tsx.name}\`, not by \`${entryPoint}\``,
+	);
+}
+
+function validateGql(file: FeatureFile): void {
+	assert(
+		file.tsx.exists(),
+		'Does not match any existing features. The filename should match the feature that uses it.',
+	);
+
+	assert(
+		file.tsx.contents().includes(`from './${file.name}';`),
+		`Should be imported by \`${file.tsx.name}\``,
+	);
+}
+
+function validateReadme(featureId: FeatureID): void {
+	const [featureMeta, duplicate] = featuresInReadme.filter(feature => feature.id === featureId);
+	assert(featureMeta, 'Should be described in the readme');
+
+	assert(
+		featureMeta.description.length >= 20,
+		'Should be described better in the readme (at least 20 characters)',
+	);
+
+	assert(
+		screenshotRegex.test(featureMeta.screenshot!)
+		|| noScreenshotExceptions.has(featureId),
+		'Should have a screenshot (png/gif) in the readme, unless really difficult to demonstrate (to be discussed in review)',
+	);
+
+	assert(!duplicate, 'Should be described only once in the readme');
+}
+
+function validateTsx(file: FeatureFile): void {
+	assert(
+		importedFeatures.includes(file.id),
+		`Should be imported by \`${entryPoint}\``,
+	);
+
+	const fileContents = readFileSync(`source/features/${file.name}`);
+
+	if (fileContents.includes('.addCssFeature')) {
+		assert(
+			!fileContents.includes('.add('),
+			`${file.id} should use either \`addCssFeature\` or \`add\`, not both`,
+		);
+
+		assert(
+			file.css.exists(),
+			`${file.id} uses \`.addCssFeature\`, but ${file.css.name} is missing`,
+		);
+
+		assert(
+			file.css.contents().includes(`[rgh-${file.id}]`),
+			`${file.css.name} should contain a \`[rgh-${file.id}]\` selector`,
+		);
+	}
+
+	if (!isFeaturePrivate(file.name)) {
+		validateReadme(file.id);
+	}
+}
+
+describe('features', async () => {
+	const featuresDirContents = readdirSync('source/features/');
+	test.each(featuresDirContents)('%s', filename => {
+		if (isGitIgnored(filename)) {
+			return;
+		}
+
+		if (filename.endsWith('.gql')) {
+			validateGql(new FeatureFile(filename));
+			return;
+		}
+
+		if (filename.endsWith('.css')) {
+			validateCss(new FeatureFile(filename));
+			return;
+		}
+
+		if (filename.endsWith('.tsx')) {
+			validateTsx(new FeatureFile(filename));
+			return;
+		}
+
+		assert.fail(`The \`/source/features\` folder should only contain .css, .tsx and .gql files. Found \`source/features/${filename}\``);
+	});
+});

build/features.test.ts

@@ -8,6 +8,11 @@ import {
 test('readme-parser', async () => {
 	await expect(getImportedFeatures().join('\n') + '\n')
 		.toMatchFileSnapshot('./__snapshots__/imported-features.txt');
-	await expect(JSON.parse(JSON.stringify(getFeaturesMeta())))
+	const featuresMetaJson = JSON.stringify(
+		getFeaturesMeta(),
+		(_, value) => value ?? null, // Convert undefined to null to make them visible in the JSON
+		'\t',
+	);
+	await expect(featuresMetaJson + '\n')
 		.toMatchFileSnapshot('./__snapshots__/features-meta.json');
 });

build/readme-parser.test.ts

@@ -7,12 +7,9 @@ import parseMarkdown from 'snarkdown';
 // Group names must be unique because they will be merged
 const simpleFeatureRegex = /^- \[]\(# "(?<simpleId>[^"]+)"\)(?: 🔥)? (?<simpleDescription>.+)$/gm;
 const highlightedFeatureRegex = /<p><a title="(?<highlightedId>[^"]+)"><\/a> (?<highlightedDescripion>.+?)\n\t+<p><img src="(?<highlightedImage>.+?)">/g;
-// eslint-disable-next-line unicorn/better-regex -- ur wrong
 const featureRegex = regexJoin(simpleFeatureRegex, /|/, highlightedFeatureRegex);
 const imageRegex = /\.\w{3}$/; // 3 since .png and .gif have 3 letters
-// eslint-disable-next-line unicorn/better-regex -- ur dably rong
 const rghUploadsRegex = /refined-github[/]refined-github[/]assets[/]/;
-// eslint-disable-next-line unicorn/better-regex -- so tripoli wron
 const screenshotRegex = regexJoin(imageRegex, /|/, rghUploadsRegex);
 
 function extractDataFromMatch(match: RegExpMatchArray): FeatureMeta {

build/readme-parser.ts

@@ -15,8 +15,7 @@
 		"lint:prettier": "prettier . --check",
 		"pack:safari": "xcodebuild -project 'safari/Refined GitHub.xcodeproj' -scheme 'Refined GitHub' -destination 'platform=macOS'",
 		"prepare:safari": "bash build/prepare-safari-release.sh",
-		"test": "run-p vitest lint:* build:* test:features --continue-on-error",
-		"test:features": "tsx build/verify-features.ts",
+		"test": "run-p vitest lint:* build:* --continue-on-error",
 		"watch": "run-p watch:* --continue-on-error",
 		"watch:typescript": "tsc --noEmit --watch --preserveWatchOutput",
 		"watch:webpack": "npm run x:webpack -- --mode=development --watch",
@@ -63,6 +62,7 @@
 		"dom-loaded": "^3.0.0",
 		"doma": "^4.0.0",
 		"element-ready": "^6.2.2",
+		"fast-ignore": "^1.1.1",
 		"filter-altered-clicks": "^2.0.1",
 		"fit-textarea": "^2.0.0",
 		"flat-zip": "^1.0.1",

build/verify-features.ts

@@ -139,7 +139,6 @@ function init(signal: AbortSignal): void {
 	observe('#repos-sticky-header', add, {signal});
 }
 
-// eslint-disable-next-line unicorn/better-regex -- No "\/"
 const featureUrlRegex = /^([/]refined-github){2}[/]blob[/][^/]+[/]source[/]features[/][^.]+[.](tsx|css)$/;
 
 void features.add(import.meta.url, {

package-lock.json

@@ -8,13 +8,13 @@ function getRepoReference(currentRepo: RepositoryInfo | undefined, repoNameWithO
 }
 
 const escapeRegex = (string: string): string => string.replaceAll(/[\\^$.*+?()[\]{}|]/g, '\\$&');
-const prCommitPathnameRegex = /[/]([^/]+[/][^/]+)[/]pull[/](\d+)[/]commits[/]([\da-f]{7})[\da-f]{33}(?:#[\w-]+)?\b/; // eslint-disable-line unicorn/better-regex
+const prCommitPathnameRegex = /[/]([^/]+[/][^/]+)[/]pull[/](\d+)[/]commits[/]([\da-f]{7})[\da-f]{33}(?:#[\w-]+)?\b/;
 export const prCommitUrlRegex = new RegExp('\\b' + escapeRegex(location.origin) + prCommitPathnameRegex.source, 'gi');
 
-const prComparePathnameRegex = /[/]([^/]+[/][^/]+)[/]compare[/](.+)(#diff-[\da-fR-]+)/; // eslint-disable-line unicorn/better-regex
+const prComparePathnameRegex = /[/]([^/]+[/][^/]+)[/]compare[/](.+)(#diff-[\da-fR-]+)/;
 export const prCompareUrlRegex = new RegExp('\\b' + escapeRegex(location.origin) + prComparePathnameRegex.source, 'gi');
 
-const discussionPathnameRegex = /[/]([^/]+[/][^/]+)[/]discussions[/](\d+)[?][^#\s]+(#[\w-]+)?\b/; // eslint-disable-line unicorn/better-regex
+const discussionPathnameRegex = /[/]([^/]+[/][^/]+)[/]discussions[/](\d+)[?][^#\s]+(#[\w-]+)?\b/;
 export const discussionUrlRegex = new RegExp('\\b' + escapeRegex(location.origin) + discussionPathnameRegex.source, 'gi');
 
 // To be used as replacer callback in string.replace() for PR commit links

package.json

@@ -1,5 +1,4 @@
 /* eslint-disable @typescript-eslint/consistent-type-definitions -- Declaration merging necessary */
-/* eslint-disable unicorn/better-regex -- Go home you're drunk */
 
 import regexJoin from 'regex-join';
 import type CodeMirror from 'codemirror';
@@ -129,4 +128,3 @@ function acceptBranch(branch: string, line: number): void {
 	editor.setCursor(linesToRemove[0]);
 }
 
-/* eslint-enable unicorn/better-regex */