Revert "Convert EmulatorConfig's HostAndPort to be an optional property"

This reverts commit c58e63b.

Author: dlarocque

packages/rules-unit-testing/src/impl/discovery.ts

@@ -84,20 +84,29 @@ export interface DiscoveredEmulators {
  */
 export function getEmulatorHostAndPort(
   emulator: keyof DiscoveredEmulators,
-  endpoint?: HostAndPort,
+  conf?: EmulatorConfig,
   discovered?: DiscoveredEmulators
-): HostAndPort | undefined {
-  if (endpoint) {
-    if (discovered && !discovered[emulator]) {
-      console.warn(
-        `Warning: config for the ${emulator} emulator is specified, but the Emulator hub ` +
-          'reports it as not running. This may lead to errors such as connection refused.'
-      );
+) {
+  if (conf && 'host' in conf && 'port' in conf) {
+    const { host, port } = conf;
+    if (host || port) {
+      if (!host || !port) {
+        throw new Error(
+          `Invalid configuration ${emulator}.host=${host} and ${emulator}.port=${port}. ` +
+            'If either parameter is supplied, both must be defined.'
+        );
+      }
+      if (discovered && !discovered[emulator]) {
+        console.warn(
+          `Warning: config for the ${emulator} emulator is specified, but the Emulator hub ` +
+            'reports it as not running. This may lead to errors such as connection refused.'
+        );
+      }
+      return {
+        host: fixHostname(conf.host, discovered?.hub?.host),
+        port: conf.port
+      };
     }
-    return {
-      host: fixHostname(endpoint.host, discovered?.hub?.host),
-      port: endpoint.port
-    };
   }
   const envVar = EMULATOR_HOST_ENV_VARS[emulator];
   const fallback = discovered?.[emulator] || emulatorFromEnvVar(envVar);

packages/rules-unit-testing/src/initialize.ts

@@ -73,7 +73,7 @@ export async function initializeTestEnvironment(
   for (const emulator of SUPPORTED_EMULATORS) {
     const hostAndPort = getEmulatorHostAndPort(
       emulator,
-      config[emulator]?.endpoint,
+      config[emulator],
       discovered
     );
     if (hostAndPort) {

packages/rules-unit-testing/src/public_types/index.ts

@@ -128,7 +128,7 @@ export interface TestEnvironmentConfig {
  * An object containing the hostname and port number of an emulator.
  * @public
  */
-export type HostAndPort = {
+export interface HostAndPort {
   /**
    * The host of the emulator. Can be omitted if discovered automatically through the hub or
    * specified via environment variables. See `TestEnvironmentConfig` for details.
@@ -140,7 +140,7 @@ export type HostAndPort = {
    * specified via environment variables. See `TestEnvironmentConfig` for details.
    */
   port: number;
-};
+}
 
 /**
  * Configuration for a given emulator.
@@ -149,12 +149,7 @@ export type HostAndPort = {
 export type EmulatorConfig = {
   /** The security rules source code under test for this emulator. Strongly recommended. */
   rules?: string;
-  /**
-   * Endpoint of the emulator. Can be omitted if discovered automatically through the hub.
-   * See `TestEnvironmentConfig` for details.
-   */
-  endpoint?: HostAndPort;
-};
+} & (HostAndPort | {}); // Both or none of host and port should be specified.
 
 /**
  * An object used to control the rules unit test environment. Can be used to create RulesTestContext

packages/rules-unit-testing/test/impl/discovery.test.ts

@@ -22,7 +22,7 @@ import {
   EMULATOR_HOST_ENV_VARS,
   getEmulatorHostAndPort
 } from '../../src/impl/discovery';
-import { EmulatorConfig, HostAndPort } from '../../src/public_types';
+import { HostAndPort } from '../../src/public_types';
 import { restoreEnvVars, stashEnvVars } from '../test_utils';
 
 describe('discoverEmulators()', () => {
@@ -106,8 +106,10 @@ describe('getEmulatorHostAndPort()', () => {
       expect(result).to.be.undefined;
     });
 
-    it('returns undefined if endpoint is undefined', async () => {
-      const result = getEmulatorHostAndPort('hub');
+    it('returns undefined if config option does not contain host/port', async () => {
+      const result = getEmulatorHostAndPort('hub', {
+        rules: '/* security rules only, no host/port */'
+      });
 
       expect(result).to.be.undefined;
     });
@@ -121,6 +123,22 @@ describe('getEmulatorHostAndPort()', () => {
       expect(result?.host).to.equal('::1');
     });
 
+    it('throws if only host is present', async () => {
+      expect(() =>
+        getEmulatorHostAndPort('hub', {
+          host: '[::1]'
+        } as HostAndPort)
+      ).to.throw(/hub.port=undefined/);
+    });
+
+    it('throws if only port is present', async () => {
+      expect(() =>
+        getEmulatorHostAndPort('database', {
+          port: 1234
+        } as HostAndPort)
+      ).to.throw(/Invalid configuration database.host=undefined/);
+    });
+
     it('connect to 127.0.0.1 if host is wildcard 0.0.0.0', async () => {
       const result = getEmulatorHostAndPort('hub', {
         host: '0.0.0.0',
@@ -286,7 +304,9 @@ describe('getEmulatorHostAndPort()', () => {
 
     it('takes host and port from env var if config has no host/port', async () => {
       process.env[EMULATOR_HOST_ENV_VARS.hub] = '127.0.0.1:3445';
-      const result = getEmulatorHostAndPort('hub');
+      const result = getEmulatorHostAndPort('hub', {
+        rules: '/* security rules only, no host/port */'
+      });
 
       expect(result?.host).to.equal('127.0.0.1');
       expect(result?.port).to.equal(3445);