Move getQuestionsForPackage to its own file (#1006)

microsoft · Nov 18, 2024 · ae9721a · ae9721a
1 parent 16d9a80
commit ae9721a
Show file tree

Hide file tree

Showing 6 changed files with 166 additions and 135 deletions.
diff --git a/change/beachball-fbe57b6e-69d7-4628-8c28-6000d9e9449b.json b/change/beachball-fbe57b6e-69d7-4628-8c28-6000d9e9449b.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Internal refactor: getQuestionsForPackage to its own file",
+  "packageName": "beachball",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/...tForChange_getQuestionsForPackage.test.ts → ...changefile/getQuestionsForPackage.test.ts b/...tForChange_getQuestionsForPackage.test.ts → ...changefile/getQuestionsForPackage.test.ts
@@ -1,19 +1,19 @@
 import { describe, expect, it, jest } from '@jest/globals';
 import prompts from 'prompts';
-import { _getQuestionsForPackage } from '../../changefile/promptForChange';
+import { getQuestionsForPackage } from '../../changefile/getQuestionsForPackage';
 import { ChangeFilePromptOptions } from '../../types/ChangeFilePrompt';
 import { initMockLogs } from '../../__fixtures__/mockLogs';
 import { makePackageInfos } from '../../__fixtures__/packageInfos';
 
 /**
  * This covers the first part of `promptForChange`: determining what questions to ask for each package.
  */
-describe('promptForChange _getQuestionsForPackage', () => {
+describe('getQuestionsForPackage', () => {
   /** Package name used in the tests */
   const pkg = 'foo';
 
-  /** Basic params for `_getQuestionsForPackage`, for a package named `foo` */
-  const defaultQuestionsParams: Parameters<typeof _getQuestionsForPackage>[0] = {
+  /** Basic params for `getQuestionsForPackage`, for a package named `foo` */
+  const defaultQuestionsParams: Parameters<typeof getQuestionsForPackage>[0] = {
     pkg,
     packageInfos: makePackageInfos({ [pkg]: {} }),
     packageGroups: {},
@@ -24,7 +24,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   const logs = initMockLogs();
 
   it('works in basic case', () => {
-    const questions = _getQuestionsForPackage(defaultQuestionsParams);
+    const questions = getQuestionsForPackage(defaultQuestionsParams);
     expect(questions).toEqual([
       {
         choices: [
@@ -50,7 +50,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
 
   // it's somewhat debatable if this is correct (maybe --type should be the override for disallowedChangeTypes?)
   it('errors if options.type is disallowed', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       packageInfos: makePackageInfos({ [pkg]: { combinedOptions: { disallowedChangeTypes: ['major'] } } }),
       options: { type: 'major', message: '' },
@@ -60,7 +60,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   });
 
   it('errors if there are no valid change types for package', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       packageInfos: makePackageInfos({
         [pkg]: { combinedOptions: { disallowedChangeTypes: ['major', 'minor', 'patch', 'none'] } },
@@ -71,7 +71,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   });
 
   it('respects disallowedChangeTypes', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       packageInfos: makePackageInfos({ [pkg]: { combinedOptions: { disallowedChangeTypes: ['major'] } } }),
     });
@@ -80,7 +80,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   });
 
   it('allows prerelease change for package with prerelease version', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       packageInfos: makePackageInfos({ [pkg]: { version: '1.0.0-beta.1' } }),
     });
@@ -90,7 +90,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
 
   // this is a bit weird as well, but documenting current behavior
   it('excludes prerelease if disallowed', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       packageInfos: makePackageInfos({
         [pkg]: { version: '1.0.0-beta.1', combinedOptions: { disallowedChangeTypes: ['prerelease'] } },
@@ -101,7 +101,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   });
 
   it('excludes the change type question when options.type is specified', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       options: { type: 'patch', message: '' },
     });
@@ -110,7 +110,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   });
 
   it('excludes the change type question with only one valid option', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       packageInfos: makePackageInfos({
         [pkg]: { combinedOptions: { disallowedChangeTypes: ['major', 'minor', 'none'] } },
@@ -121,7 +121,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   });
 
   it('excludes the change type question when prerelease is implicitly the only valid option', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       packageInfos: makePackageInfos({
         [pkg]: {
@@ -135,7 +135,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   });
 
   it('excludes the comment question when options.message is set', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       options: { message: 'message' },
     });
@@ -147,7 +147,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
     const customQuestions: prompts.PromptObject[] = [{ name: 'custom', message: 'custom prompt', type: 'text' }];
     const changePrompt: ChangeFilePromptOptions['changePrompt'] = jest.fn(() => customQuestions);
 
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       packageInfos: makePackageInfos({ [pkg]: { combinedOptions: { changeFilePrompt: { changePrompt } } } }),
     });
@@ -166,7 +166,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   it('does case-insensitive filtering on description suggestions', async () => {
     const recentMessages = ['Foo', 'Bar', 'Baz'];
     const recentMessageChoices = [{ title: 'Foo' }, { title: 'Bar' }, { title: 'Baz' }];
-    const questions = _getQuestionsForPackage({ ...defaultQuestionsParams, recentMessages });
+    const questions = getQuestionsForPackage({ ...defaultQuestionsParams, recentMessages });
     expect(questions).toEqual([
       expect.anything(),
       expect.objectContaining({

diff --git a/src/__tests__/changefile/promptForChange.test.ts b/src/__tests__/changefile/promptForChange.test.ts
@@ -3,7 +3,6 @@ import prompts from 'prompts';
 import {
   promptForChange,
   _getChangeFileInfoFromResponse,
-  _getQuestionsForPackage,
   _promptForPackageChange,
 } from '../../changefile/promptForChange';
 import { ChangeFilePromptOptions } from '../../types/ChangeFilePrompt';
@@ -16,15 +15,17 @@ import { makePackageInfos } from '../../__fixtures__/packageInfos';
 // so instead we inject a custom mock stdout stream, as well as stdin for entering answers
 let stdin: MockStdin;
 let stdout: MockStdout;
-jest.mock(
-  'prompts',
-  () =>
-    ((questions, options) => {
-      questions = Array.isArray(questions) ? questions : [questions];
-      questions = questions.map(q => ({ ...q, stdin, stdout }));
-      return (jest.requireActual('prompts') as typeof prompts)(questions, options);
-    }) as typeof prompts
-);
+jest.mock('prompts', () => {
+  const realPrompts = jest.requireActual('prompts') as typeof prompts;
+
+  return ((questions, options) => {
+    const questionsArr = Array.isArray(questions) ? questions : [questions];
+    return realPrompts(
+      questionsArr.map(q => ({ ...q, stdin, stdout })),
+      options
+    );
+  }) as typeof prompts;
+});
 
 /**
  * These combined tests mainly cover various early bail-out cases (the only meaningful logic in

diff --git a/src/__tests__/changefile/promptForChange_promptForPackageChange.test.ts b/src/__tests__/changefile/promptForChange_promptForPackageChange.test.ts
@@ -1,24 +1,27 @@
 import { afterEach, beforeEach, describe, expect, it, jest } from '@jest/globals';
 import prompts from 'prompts';
-import { _getQuestionsForPackage, _promptForPackageChange } from '../../changefile/promptForChange';
+import { _promptForPackageChange } from '../../changefile/promptForChange';
 import { initMockLogs } from '../../__fixtures__/mockLogs';
 import { MockStdin } from '../../__fixtures__/mockStdin';
 import { MockStdout } from '../../__fixtures__/mockStdout';
 import { makePackageInfos } from '../../__fixtures__/packageInfos';
+import { getQuestionsForPackage } from '../../changefile/getQuestionsForPackage';
 
 // prompts writes to stdout (not console) in a way that can't really be mocked with spies,
 // so instead we inject a custom mock stdout stream, as well as stdin for entering answers
 let stdin: MockStdin;
 let stdout: MockStdout;
-jest.mock(
-  'prompts',
-  () =>
-    ((questions, options) => {
-      questions = Array.isArray(questions) ? questions : [questions];
-      questions = questions.map(q => ({ ...q, stdin, stdout }));
-      return (jest.requireActual('prompts') as typeof prompts)(questions, options);
-    }) as typeof prompts
-);
+jest.mock('prompts', () => {
+  const realPrompts = jest.requireActual('prompts') as typeof prompts;
+
+  return ((questions, options) => {
+    const questionsArr = Array.isArray(questions) ? questions : [questions];
+    return realPrompts(
+      questionsArr.map(q => ({ ...q, stdin, stdout })),
+      options
+    );
+  }) as typeof prompts;
+});
 
 /**
  * This covers the actual prompting part of `promptForChange`.
@@ -27,8 +30,8 @@ describe('promptForChange _promptForPackageChange', () => {
   /** Package name used in the tests */
   const pkg = 'foo';
 
-  /** Basic params for `_getQuestionsForPackage`, for a package named `foo` */
-  const defaultQuestionsParams: Parameters<typeof _getQuestionsForPackage>[0] = {
+  /** Basic params for `getQuestionsForPackage`, for a package named `foo` */
+  const defaultQuestionsParams: Parameters<typeof getQuestionsForPackage>[0] = {
     pkg,
     packageInfos: makePackageInfos({ [pkg]: {} }),
     packageGroups: {},
@@ -64,7 +67,7 @@ describe('promptForChange _promptForPackageChange', () => {
   });
 
   it('prompts for change type and description', async () => {
-    const questions = _getQuestionsForPackage(defaultQuestionsParams);
+    const questions = getQuestionsForPackage(defaultQuestionsParams);
     expect(questions).toEqual(expectedQuestions);
 
     const answersPromise = _promptForPackageChange(questions!, pkg);
@@ -90,7 +93,7 @@ describe('promptForChange _promptForPackageChange', () => {
 
   it('accepts custom description typed by character', async () => {
     // For this one we provide a type in options and only ask for the description
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       options: { type: 'minor', message: '' },
     });
@@ -121,7 +124,7 @@ describe('promptForChange _promptForPackageChange', () => {
 
   it('accepts custom description pasted', async () => {
     // For this one we provide a type in options and only ask for the description
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       options: { type: 'minor', message: '' },
     });
@@ -151,7 +154,7 @@ describe('promptForChange _promptForPackageChange', () => {
 
   it('accepts custom description pasted with newline', async () => {
     // For this one we provide a type in options and only ask for the description
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       options: { type: 'minor', message: '' },
     });
@@ -177,7 +180,7 @@ describe('promptForChange _promptForPackageChange', () => {
 
   it('uses options selected with arrow keys', async () => {
     const recentMessages = ['first', 'second', 'third'];
-    const questions = _getQuestionsForPackage({ ...defaultQuestionsParams, recentMessages });
+    const questions = getQuestionsForPackage({ ...defaultQuestionsParams, recentMessages });
     expect(questions).toEqual(expectedQuestions);
 
     const answerPromise = _promptForPackageChange(questions!, pkg);
@@ -223,7 +226,7 @@ describe('promptForChange _promptForPackageChange', () => {
   });
 
   it('filters options while typing', async () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       recentMessages: ['foo', 'bar', 'baz'],
       options: { type: 'minor', message: '' },
@@ -254,7 +257,7 @@ describe('promptForChange _promptForPackageChange', () => {
   });
 
   it('handles pressing delete while typing', async () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       recentMessages: ['foo', 'bar', 'baz'],
       options: { type: 'minor', message: '' },
@@ -288,7 +291,7 @@ describe('promptForChange _promptForPackageChange', () => {
   });
 
   it('returns no answers if cancelled with ctrl-c', async () => {
-    const questions = _getQuestionsForPackage(defaultQuestionsParams);
+    const questions = getQuestionsForPackage(defaultQuestionsParams);
     expect(questions).toEqual(expectedQuestions);
 
     const answerPromise = _promptForPackageChange(questions!, pkg);