feat: add os-specific bisect to bot

modules/bot/src/broker-client.ts

@@ -35,6 +35,7 @@ export default class BrokerAPI {
       gist: command.gistId,
       history: [],
       id: mkuuid(),
+      platform: command.platform,
       time_added: Date.now(),
       type: JobType.bisect,
       version_range: [command.goodVersion, command.badVersion],

modules/bot/src/issue-parser.ts

@@ -8,7 +8,10 @@ import { inspect } from 'util';
 
 import { Versions, compareVersions } from 'fiddle-core';
 
-import { Platform } from '@electron/bugbot-shared/build/interfaces';
+import {
+  ALL_PLATFORMS,
+  Platform,
+} from '@electron/bugbot-shared/build/interfaces';
 
 // no types exist for this module
 //eslint-disable-next-line @typescript-eslint/no-var-requires
@@ -19,6 +22,7 @@ export type BisectCommand = {
   gistId: string;
   goodVersion: string;
   type: 'bisect';
+  platform: Platform;
 };
 
 export type TestCommand = {
@@ -37,8 +41,11 @@ export function splitMarkdownByHeader(markdown: string): Map<string, string> {
   for (const child of tree.children) {
     if (child.type === 'heading') {
       const headerName = child.children[0].value as string;
+      // Need to escape the header name here because the string input gets put into a RegExp value
+      // https://stackoverflow.com/a/3561711/5602134
+      const escapedHeader = headerName.replace(/[-/\\^$*+?.()|[\]{}]/g, '\\$&');
       let content = '';
-      heading(tree, headerName, (_start: Heading, nodes: Array<Node>) => {
+      heading(tree, escapedHeader, (_start: Heading, nodes: Array<Node>) => {
         content = toString(nodes);
       });
       sections.set(headerName, content.trim());
@@ -63,14 +70,39 @@ export function getGistId(input?: string): string | null {
   return null;
 }
 
-const BAD_VERSION = 'Electron Version';
-const GOOD_VERSION = 'Last Known Working Electron Version';
-const TESTCASE_URL = 'Testcase Gist URL';
+export function getPlatform(maybePlatform: string): Platform | undefined {
+  const lowercasePlatform = maybePlatform.toLowerCase();
+  const platformMatches: Map<Platform, string[]> = new Map();
+  platformMatches.set('darwin', ['macos', 'mac', 'osx']);
+  platformMatches.set('linux', ['linux', 'ubuntu']);
+  platformMatches.set('win32', ['windows']);
 
-// /bugbot bisect [gistId] [goodVersion [badVersion]]
+  for (const [platform, matches] of platformMatches.entries()) {
+    if (lowercasePlatform.includes(platform)) return platform;
+    for (const match of matches) {
+      if (lowercasePlatform.includes(match)) return platform;
+    }
+  }
+
+  return undefined;
+}
+
+const ISSUE_SECTIONS = {
+  badVersion: 'Electron Version',
+  goodVersion: 'Last Known Working Electron Version',
+  gistId: 'Testcase Gist URL',
+  platform: 'What operating system are you using?',
+};
+
+export function isValidPlatform(value: string): value is Platform {
+  return ALL_PLATFORMS.includes(value as Platform);
+}
+
+// /bugbot bisect [gistId] [goodVersion [badVersion]] [platform]
 // If no `gistId` is given, use TESTCASE_URL.
 // If no `goodVersion` is given, use GOOD_VERSION or an old version.
 // If no `badVersion` is given, use BAD_VERSION or the latest release.
+// If no `platform` is given, use the platform from the reported issue or Linux.
 function parseBisectCommand(
   issueBody: string,
   words: string[],
@@ -81,6 +113,7 @@ function parseBisectCommand(
   let badVersion: semver.SemVer | undefined;
   let gistId: string | undefined;
   let goodVersion: semver.SemVer | undefined;
+  let platform: Platform | undefined;
 
   for (const word of words) {
     const id = getGistId(word);
@@ -97,16 +130,22 @@ function parseBisectCommand(
       }
       continue;
     }
+    const plat = getPlatform(word);
+    if (isValidPlatform(plat)) {
+      platform = plat;
+      continue;
+    }
   }
 
   // if any pieces are missing, fill them in from the issue body
   const sections = splitMarkdownByHeader(issueBody);
   d('sections', inspect(sections));
-  badVersion ||= semver.coerce(sections.get(BAD_VERSION));
+  badVersion ||= semver.coerce(sections.get(ISSUE_SECTIONS.badVersion));
   badVersion ||= versions.latest;
-  goodVersion ||= semver.coerce(sections.get(GOOD_VERSION));
+  goodVersion ||= semver.coerce(sections.get(ISSUE_SECTIONS.goodVersion));
   goodVersion ||= semver.parse(`${versions.supportedMajors[0] - 2}.0.0`);
-  gistId ||= getGistId(sections.get(TESTCASE_URL));
+  gistId ||= getGistId(sections.get(ISSUE_SECTIONS.gistId));
+  platform ||= getPlatform(sections.get(ISSUE_SECTIONS.platform)) ?? 'linux';
 
   // ensure goodVersion < badVersion;
   const semGood = semver.parse(goodVersion);
@@ -115,18 +154,18 @@ function parseBisectCommand(
     [goodVersion, badVersion] = [badVersion, goodVersion];
   }
 
-  d('%o', { badVersion, gistId, goodVersion });
+  d('%o', { badVersion, gistId, goodVersion, platform });
   return badVersion && gistId && goodVersion
     ? {
         badVersion: badVersion?.version,
         gistId,
         goodVersion: goodVersion?.version,
+        platform,
         type: 'bisect',
       }
     : undefined;
 }
 
-const ALL_PLATFORMS = ['darwin', 'linux', 'win32'];
 const NUM_OBSOLETE_TO_TEST = 2;
 
 function getVersionsToTest(versions: Versions): Array<string> {
@@ -170,8 +209,8 @@ function parseTestCommand(
       ret.gistId = id;
       continue;
     }
-    if (ALL_PLATFORMS.includes(word)) {
-      ret.platforms.push(word as Platform);
+    if (isValidPlatform(word)) {
+      ret.platforms.push(word);
       continue;
     }
     const ver = semver.coerce(word);
@@ -187,10 +226,10 @@ function parseTestCommand(
     ret.versions.push(...getVersionsToTest(versions));
   }
   if (ret.platforms.length === 0) {
-    ret.platforms.push(...(ALL_PLATFORMS as Platform[]));
+    ret.platforms.push(...ALL_PLATFORMS);
   }
   if (!ret.gistId) {
-    ret.gistId = getGistId(sections.get(TESTCASE_URL));
+    ret.gistId = getGistId(sections.get(ISSUE_SECTIONS.gistId));
   }
 
   d('after filling in defaults: %o', ret);

modules/bot/test/issue-parser.spec.ts

@@ -175,6 +175,7 @@ describe('issue-parser', () => {
         badVersion: '13.0.0',
         gistId: fixtureGistId,
         goodVersion: '12.0.0',
+        platform: 'win32',
         type: 'bisect',
       };
 
@@ -183,7 +184,7 @@ describe('issue-parser', () => {
         expect(otherBadVersion).not.toBe(expectedCommand.badVersion);
       });
 
-      it('finds a gist and version range', () => {
+      it('finds a gist, version range, and platform', () => {
         const issueBody = getIssueBody('issue.md');
         const command = parseIssueCommand(issueBody, COMMENT, versions);
         expect(command).toStrictEqual(expectedCommand);
@@ -254,6 +255,33 @@ describe('issue-parser', () => {
         });
       });
 
+      it('reads a platform from the issue comment', () => {
+        const issueBody = getIssueBody('issue.md');
+        const comment = `${COMMENT} darwin`;
+        const command = parseIssueCommand(issueBody, comment, versions);
+        expect(command).toStrictEqual({
+          ...expectedCommand,
+          platform: 'darwin',
+        });
+      });
+
+      it.each([
+        ['linux', 'Ubuntu'],
+        ['darwin', 'OSX'],
+        ['win32', 'Windows'],
+      ])(
+        'can match an approximate platform name for %s in the issue comment',
+        (platform, approximateName) => {
+          const issueBody = getIssueBody('issue.md');
+          const comment = `${COMMENT} ${approximateName}`;
+          const command = parseIssueCommand(issueBody, comment, versions);
+          expect(command).toStrictEqual({
+            ...expectedCommand,
+            platform,
+          });
+        },
+      );
+
       it('ignores inscrutable comment arguments', () => {
         const issueBody = getIssueBody('issue.md');
         const comment = `${COMMENT} fnord`;
@@ -279,7 +307,7 @@ describe('issue-parser', () => {
         expect(command).toStrictEqual({
           ...expectedCommand,
           goodVersion: fakeBisectStart,
-          badVersion: fakeLatestVersion
+          badVersion: fakeLatestVersion,
         });
       });
 

modules/shared/src/interfaces.ts

@@ -16,8 +16,13 @@ const JobIdPredicate = ow.string.validate((str) => ({
   message: `Expected value to be a UUID, got ${str}`,
 }));
 
-export type Platform = 'darwin' | 'linux' | 'win32';
-const PlatformPredicate = ow.string.oneOf(['darwin', 'linux', 'win32']);
+// Editing `PLATFORM_LIST` should automatically update the constants after it,
+// including the `Platform` type thanks to some type magic :)
+const PLATFORM_LIST = ['darwin', 'linux', 'win32'] as const;
+
+export type Platform = typeof PLATFORM_LIST[number];
+export const ALL_PLATFORMS: readonly Platform[] = PLATFORM_LIST;
+const PlatformPredicate = ow.string.oneOf(ALL_PLATFORMS);
 
 export type RunnerId = string;
 const RunnerIdPredicate = ow.string;