Fix tests for Doctor (#677)

The tests in DoctorSpec seemed to be causing the test suite to abort, and broken tests weren't being properly reported. My guess is that many of the tests in DoctorSpec have actually never worked. This PR fixes DoctorSpec and other tests that were broken.
GoogleChromeLabs · Apr 25, 2022 · 640b1f1 · 640b1f1
1 parent 8936e33
commit 640b1f1
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 77 deletions.
diff --git a/packages/cli/src/lib/cmds/doctor.ts b/packages/cli/src/lib/cmds/doctor.ts
@@ -43,8 +43,9 @@ async function androidSdkDoctor(config: Config, log: Log): Promise<boolean> {
  return true;
 }
 
-export async function doctor(log: Log = new ConsoleLog('doctor')): Promise<boolean> {
- const config = await loadOrCreateConfig(log);
+export async function doctor(
+ log: Log = new ConsoleLog('doctor'), configPath?: string | undefined): Promise<boolean> {
+ const config = await loadOrCreateConfig(log, undefined, configPath);
  const jdkResult = await jdkDoctor(config, log);
  const androidSdkResult = await androidSdkDoctor(config, log);
  if (jdkResult && androidSdkResult) {

diff --git a/packages/cli/src/spec/DoctorSpec.ts b/packages/cli/src/spec/DoctorSpec.ts
@@ -14,63 +14,29 @@
  * limitations under the License.
  */
 
-import {loadOrCreateConfig} from '../lib/config';
 import * as mock from 'mock-fs';
-import {updateConfig} from '../lib/cmds/updateConfig';
 import {MockLog} from '@bubblewrap/core';
-import minimist = require('minimist');
 import {doctor} from '../lib/cmds/doctor';
-import {MockPrompt} from './mock/MockPrompt';
 import {enUS as messages} from '../lib/strings';
 
 describe('doctor', () => {
- let originalProcess: NodeJS.Process;
-
- beforeEach(() => {
- // Set process to a linux platform so we can run the test against expected paths on MacOS and
- // Windows.
- process = {
- platform: 'linux',
- env: {
- 'PATH': '',
- },
- } as unknown as NodeJS.Process;
- });
-
- afterEach(() => {
- // Resets to the original process to avoid breaking other tests.
- process = originalProcess;
- });
-
  describe('#jdkDoctor', () => {
  it('checks that the expected error message is sent in case that the path given isn\'t' +
  ' valid', async () => {
  // Creates a mock file system.
  mock({
- 'old/path/to/jdk': {
- 'release': 'JAVA_VERSION="1.8.3',
- },
- 'old/path/to/sdk': {
+ 'path/to/sdk': {
  'tools': {},
  },
- 'new/path/to/jdk': {},
+ 'path/to/config': '{"jdkPath":"path/to/jdk","androidSdkPath":"path/to/sdk"}',
  });
+
  const mockLog = new MockLog();
- const mockPrompt = new MockPrompt();
- // Since 'createConfig' will be called, we push 3 future answers to 'mockPrompt'.
- mockPrompt.addMessage('false'); // Should bubblewrap download the JDK?
- mockPrompt.addMessage('old/path/to/jdk'); // The path of the jdk.
- mockPrompt.addMessage('old/path/to/sdk'); // The path of the androidSdk.
- // Create config file.
- await loadOrCreateConfig(mockLog, mockPrompt);
- expect(await doctor(mockLog)).toBeTrue();
- // Change the jdkPath of the config file to a path which isn't a valid jdkPath.
- const parsedMockArgs = minimist(['--jdkPath', 'new/path/to/jdk']);
- await updateConfig(parsedMockArgs, mockLog);
- expect(await doctor(mockLog)).toBeFalse();
+ await expectAsync(doctor(mockLog, 'path/to/config')).toBeResolvedTo(false);
  // Check that the correct message was sent.
  const logErrors: Array<string> = mockLog.getReceivedData();
- expect(logErrors[logErrors.length - 1]).toEqual(messages.jdkPathIsNotCorrect);
+ const lastMessage = logErrors[logErrors.length - 1];
+ expect(lastMessage.indexOf('jdkPath isn\'t correct')).toBeGreaterThanOrEqual(0);
  mock.restore();
  });
 
@@ -84,25 +50,15 @@ describe('doctor', () => {
  'path/to/sdk': {
  'tools': {},
  },
- 'new/path/to/jdk': {
- 'release': 'JAVA_VERSION="1.7',
- },
+ 'path/to/config': '{"jdkPath":"path/to/jdk","androidSdkPath":"path/to/sdk"}',
  });
+
  const mockLog = new MockLog();
- const mockPrompt = new MockPrompt();
- // Since 'createConfig' will be called, we push 3 future answers to 'mockPrompt'.
- mockPrompt.addMessage('false'); // Should bubblewrap download the JDK?
- mockPrompt.addMessage('path/to/jdk'); // The path of the jdk.
- mockPrompt.addMessage('path/to/sdk'); // The path of the androidSdk.
- // Create config file.
- await loadOrCreateConfig(mockLog, mockPrompt);
- // Change the jdkPath of the config file to a path which isn't a supported jdkPath.
- const parsedMockArgs = minimist(['--jdkPath', 'new/path/to/jdk']);
- await updateConfig(parsedMockArgs, mockLog);
- expect(await doctor(mockLog)).toBeFalse();
+ await expectAsync(doctor(mockLog, 'path/to/config')).toBeResolvedTo(false);
  // Check that the correct message was sent.
  const logErrors: Array<string> = mockLog.getReceivedData();
- expect(logErrors[logErrors.length - 1]).toEqual(messages.jdkIsNotSupported);
+ const lastMessage = logErrors[logErrors.length - 1];
+ expect(lastMessage.indexOf('Unsupported jdk version')).toBeGreaterThanOrEqual(0);
  mock.restore();
  });
  });
@@ -112,26 +68,16 @@ describe('doctor', () => {
  ' valid', async () => {
  // Creates a mock file system.
  mock({
- 'old/path/to/jdk': {
- 'release': 'JAVA_VERSION="1.8.3',
- },
- 'old/path/to/sdk': {
- 'tools': {},
+ 'path/to/jdk': {
+ 'release': 'JAVA_VERSION="1.8',
  },
- 'new/path/to/sdk': {},
+ 'path/to/config': '{"jdkPath":"path/to/jdk","androidSdkPath":"path/to/sdk"}',
  });
+
  const mockLog = new MockLog();
- const mockPrompt = new MockPrompt();
- // Since 'createConfig' will be called, we push 3 future answers to 'mockPrompt'.
- mockPrompt.addMessage('false'); // Should bubblewrap download the JDK?
- mockPrompt.addMessage('old/path/to/jdk'); // The path of the jdk.
- mockPrompt.addMessage('old/path/to/sdk'); // The path of the androidSdk.
- // Create config file.
- await loadOrCreateConfig(mockLog, mockPrompt);
- // Change the androidSdkPath of the config file to a path which isn't a valid one.
- const parsedMockArgs = minimist(['--androidSdkPath', 'new/path/to/sdk']);
- await updateConfig(parsedMockArgs, mockLog);
- expect(await doctor(mockLog)).toBeFalse();
+
+ await expectAsync(doctor(mockLog, 'path/to/config')).toBeResolvedTo(false);
+
  // Check that the correct message was sent.
  const logErrors: Array<string> = mockLog.getReceivedData();
  expect(logErrors[logErrors.length - 1]).toEqual(messages.androidSdkPathIsNotCorrect);

diff --git a/packages/cli/src/spec/configSpec.ts b/packages/cli/src/spec/configSpec.ts
@@ -103,7 +103,7 @@ describe('config', () => {
  },
  [DEFAULT_CONFIG_FOLDER]: {
  'config.json':
- '{"content":"some old content",' +
+ '{"content":"some new content",' +
  ' "jdkPath":"/path/to/jdk","androidSdkPath":"/path/to/android-sdk"}',
  }});
  const mockLog = new MockLog();
@@ -114,8 +114,8 @@ describe('config', () => {
  // checks if the contents of the files didn't change.
  const file1 = await fsPromises.readFile(LEGACY_CONFIG_FILE_PATH, 'utf8');
  const file2 = await fsPromises.readFile(DEFAULT_CONFIG_FILE_PATH, 'utf8');
- expect(file1).toEqual('{\"content\":\"some old content\"}');
- expect(file2).toEqual('{\"content\":\"some new content\"}');
+ expect(file1.indexOf('old content')).toBeGreaterThanOrEqual(0);
+ expect(file2.indexOf('new content')).toBeGreaterThanOrEqual(0);
  mock.restore();
  });
  });