Merge pull request #339 from pact-foundation/fix/ignore-some-args

Fix/ignore some args

Author: TimothyJones

src/ffi/argumentMapper/index.ts

@@ -1,22 +1,21 @@
 import url = require('url');
 
-import { ArgMapping } from '../ffi/argumentMapper/types';
-import { ConsumerVersionSelector, VerifierOptions } from './types';
+import { ArgMapping, IgnoreOptionCombinations } from './types';
+import {
+  ConsumerVersionSelector,
+  InternalPactVerifierOptions,
+  VerifierOptions,
+} from '../types';
 
-import { getUriType } from './filesystem';
-import { LogLevel } from '../logger/types';
+import { getUriType } from '../filesystem';
+import { LogLevel } from '../../logger/types';
 
-type DeprecatedVerifierOptions = {
-  format?: 'json' | 'xml' | 'progress' | 'RspecJunitFormatter';
-  out?: string;
-  customProviderHeaders?: string[];
-  verbose?: boolean;
-  monkeypatch?: string;
-  logDir?: string;
-};
-
-// These are arguments that are on the PactJS object that we don't need to use
-export const ignoredArguments = [
+/**
+ * These are arguments that are on the PactJS object that we don't need to use
+ * An array of strings for options to ignore (typed as strings and not `keyof VerifierOptions`,
+ * because pact-js puts extra options on the object that aren't in the core VerifierOptions)
+ */
+export const ignoredArguments: Array<string> = [
   'requestFilter',
   'stateHandlers',
   'messageProviders',
@@ -26,9 +25,15 @@ export const ignoredArguments = [
   'validateSSL',
 ];
 
-export const argMapping: ArgMapping<
-  VerifierOptions & DeprecatedVerifierOptions
-> = {
+export const ignoreOptionCombinations: IgnoreOptionCombinations<VerifierOptions> =
+  {
+    enablePending: { ifNotSet: 'pactBrokerUrl' },
+    consumerVersionSelectors: { ifNotSet: 'pactBrokerUrl' },
+    consumerVersionTags: { ifNotSet: 'pactBrokerUrl' },
+    publishVerificationResult: { ifNotSet: 'pactBrokerUrl' },
+  };
+
+export const argMapping: ArgMapping<InternalPactVerifierOptions> = {
   providerBaseUrl: (providerBaseUrl: string) => {
     const u = url.parse(providerBaseUrl);
     return u && u.port && u.hostname

src/ffi/argumentMapper/types.ts

@@ -0,0 +1,88 @@
+import { FunctionMapping } from './types';
+import logger from '../../logger';
+import { InternalPactVerifierOptions } from '../types';
+import {
+  argMapping,
+  ignoredArguments,
+  ignoreOptionCombinations,
+} from './arguments';
+
+/**
+ * This function maps arguments from the Verifier to the Rust core's verifier arguments
+ *
+ * @internal
+ *
+ * @param options The actual options passed by the user
+ * @returns An array of strings to past to the Rust core verifier
+ */
+export const argumentMapper = (
+  options: InternalPactVerifierOptions
+): string[] =>
+  (Object.keys(options) as Array<keyof InternalPactVerifierOptions>)
+    .filter((k) => {
+      const ignoreCondition = ignoreOptionCombinations[k];
+      if (ignoreCondition !== undefined) {
+        logger.trace(
+          `The argument mapper has an ignored combination for '${k}'`
+        );
+        // We might have multiple ways to ignore option combinations in the future
+        if (
+          ignoreCondition.ifNotSet &&
+          options[ignoreCondition.ifNotSet] === undefined
+        ) {
+          logger.warn(
+            `Ignoring option '${k}' because it is invalid without '${ignoreCondition.ifNotSet}' also being set. This may indicate an error in your configuration`
+          );
+          return false;
+        }
+        logger.trace(`But it was not ignored: '${k}'`);
+      }
+      return true;
+    })
+    .map((key: keyof InternalPactVerifierOptions) => {
+      // We pull these out, because TypeScript doesn't like to
+      // reason that argMapping[key] is a constant value.
+      // So, we need to pull it out to be able to say
+      // `if('someKey' in thisMapping)` and retain type checking
+      const thisMapping = argMapping[key];
+      const thisValue = options[key];
+
+      if (!thisMapping) {
+        if (!ignoredArguments.includes(key)) {
+          logger.error(`Pact-core is ignoring unknown option '${key}'`);
+        }
+        return [];
+      }
+      if (thisValue === undefined) {
+        logger.warn(
+          `The Verifier option '${key}' was was explicitly set to undefined and will be ignored. This may indicate an error in your config. Remove the option entirely to prevent this warning`
+        );
+        return [];
+      }
+      if ('warningMessage' in thisMapping) {
+        logger.warn(thisMapping.warningMessage);
+        return [];
+      }
+      if ('arg' in thisMapping) {
+        switch (thisMapping.mapper) {
+          case 'string':
+            return [thisMapping.arg, `${thisValue}`];
+          case 'flag':
+            return thisValue ? [thisMapping.arg] : [];
+          default:
+            logger.pactCrash(
+              `Option mapper for '${key}' maps to '${thisMapping.arg}' with unknown mapper type '${thisMapping.mapper}'`
+            );
+            return [];
+        }
+      }
+      if (typeof thisMapping === 'function') {
+        return (thisMapping as FunctionMapping<unknown>)(thisValue);
+      }
+      logger.pactCrash(
+        `The option mapper completely failed to find a mapping for '${key}'.`
+      );
+      return [];
+    })
+    // This can be replaced with .flat() when node 10 is EOL
+    .reduce((acc: string[], current: string[]) => [...acc, ...current], []);

src/verifier/arguments.ts renamed to src/verifier/argumentMapper/arguments.ts

@@ -0,0 +1,24 @@
+type BasicMapping = {
+  arg: string;
+  mapper: 'string' | 'flag';
+};
+
+type IgnoreAndWarn = {
+  warningMessage: string;
+};
+
+/** @internal */
+export type FunctionMapping<T> = (arg: NonNullable<T>) => string[];
+
+/** @internal */
+export type ArgMapping<PactOptions> = {
+  [Key in keyof PactOptions]-?:
+    | BasicMapping
+    | IgnoreAndWarn
+    | FunctionMapping<PactOptions[Key]>;
+};
+
+/** @internal */
+export type IgnoreOptionCombinations<T> = {
+  [Key in keyof Partial<T>]: { ifNotSet: keyof T };
+};

src/verifier/argumentMapper/index.ts

@@ -1,8 +1,7 @@
 import { VerifierOptions } from './types';
 import { getFfiLib } from '../ffi';
-import logger from '../logger';
-import { argMapping, ignoredArguments } from './arguments';
-import { argumentMapper } from '../ffi/argumentMapper';
+import logger, { setLogLevel } from '../logger';
+import { argumentMapper } from './argumentMapper';
 
 const VERIFICATION_SUCCESSFUL = 0;
 const VERIFICATION_FAILED = 1;
@@ -12,9 +11,12 @@ const INVALID_ARGUMENTS = 4;
 
 export const verify = (opts: VerifierOptions): Promise<string> => {
   const verifierLib = getFfiLib(opts.logLevel);
+  if (opts.logLevel) {
+    setLogLevel(opts.logLevel);
+  }
   // Todo: probably separate out the sections of this logic into separate promises
   return new Promise<string>((resolve, reject) => {
-    const request = argumentMapper(argMapping, opts, ignoredArguments)
+    const request = argumentMapper(opts)
       .map((s) => s.replace('\n', ''))
       .join('\n');
 

src/verifier/argumentMapper/types.ts

@@ -31,3 +31,21 @@ export interface VerifierOptions {
   logLevel?: LogLevel;
   disableSslVerification?: boolean;
 }
+
+/** These are the deprecated verifier options, removed prior to this verison,
+ * but it's useful to know what they were so we can potentially map or warn.
+ */
+type DeprecatedVerifierOptions = {
+  format?: 'json' | 'xml' | 'progress' | 'RspecJunitFormatter';
+  out?: string;
+  customProviderHeaders?: string[];
+  verbose?: boolean;
+  monkeypatch?: string;
+  logDir?: string;
+};
+
+/** Helper type for the mapper to reason about the options we want to be able to handle.
+ * Not exposed, because we only want to expose the current VerifierOptions to the user
+ * @internal */
+export type InternalPactVerifierOptions = VerifierOptions &
+  DeprecatedVerifierOptions;

src/verifier/nativeVerifier.ts

@@ -46,6 +46,26 @@ describe('Verifier Integration Spec', () => {
             ],
           }).verify()
         ).to.eventually.be.fulfilled);
+      context('with some broker args but no broker URL', () => {
+        it('should return a successful promise', () =>
+          expect(
+            verifierFactory({
+              ...DEFAULT_ARGS,
+              providerBaseUrl: providerBaseUrl,
+              pactUrls: [
+                path.resolve(
+                  __dirname,
+                  'integration/me-they-weird path-success.json'
+                ),
+              ],
+              // These don't mean anything without a broker URL, but should not fail the verification
+              enablePending: true,
+              consumerVersionSelectors: [{ latest: true }],
+              consumerVersionTags: ['main'],
+              publishVerificationResult: true,
+            }).verify()
+          ).to.eventually.be.fulfilled);
+      });
     });
 
     context('without provider states', () => {

src/verifier/types.ts

@@ -16,7 +16,8 @@
     "skipLibCheck": true,
     "strictNullChecks": true,
     "strict": true,
-    "noUnusedLocals": true
+    "noUnusedLocals": true,
+    "stripInternal": true
   },
   "include": ["**/*.ts"],
   "exclude": ["node_modules", "**/*.spec.ts", "**/__mocks__"],