diff --git a/modules/bot/src/broker-client.ts b/modules/bot/src/broker-client.ts index d398cf2..08d11eb 100644 --- a/modules/bot/src/broker-client.ts +++ b/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], diff --git a/modules/bot/src/issue-parser.ts b/modules/bot/src/issue-parser.ts index b297b6d..dfbd026 100644 --- a/modules/bot/src/issue-parser.ts +++ b/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 { 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) => { + heading(tree, escapedHeader, (_start: Heading, nodes: Array) => { 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 = 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 { @@ -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); diff --git a/modules/bot/test/issue-parser.spec.ts b/modules/bot/test/issue-parser.spec.ts index aa5766b..93766c2 100644 --- a/modules/bot/test/issue-parser.spec.ts +++ b/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, }); }); diff --git a/modules/shared/src/interfaces.ts b/modules/shared/src/interfaces.ts index 954c344..1513b92 100644 --- a/modules/shared/src/interfaces.ts +++ b/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;