Make it clearer what the expectations for isUsernamePassword are

Author: mbg

lib/start-proxy-action.js

@@ -1,5 +1,15 @@
+import { ExecutionContext } from "ava";
+
 import * as json from ".";
 
+/**
+ * Constructs an object based on `schema` for unit tests.
+ * Assumes that all keys in `schema` have string values.
+ *
+ * @param includeOptional Whether to include optional properties.
+ * @param schema The schema to base the object on.
+ * @returns An object that satisfies `schema`.
+ */
 export function makeFromSchema<S extends json.Schema>(
   includeOptional: boolean,
   schema: S,
@@ -13,3 +23,75 @@ export function makeFromSchema<S extends json.Schema>(
   }
   return result as json.FromSchema<S>;
 }
+
+/** Options for `withSchemaMatrix`. */
+export interface SchemaMatrixOptions {
+  /** Whether cases where the properties are entirely absent should be excluded. */
+  excludeAbsent?: boolean;
+}
+
+/**
+ * Constructs a test matrix of possible objects for `schema`: all required properties
+ * plus all permutations of possible states for the optional properties.
+ *
+ * @param schema The schema to construct a test matrix for.
+ * @param body The test body to call with each value from the test matrix.
+ */
+export function withSchemaMatrix<S extends json.Schema>(
+  t: ExecutionContext<any>,
+  schema: S,
+  opts: SchemaMatrixOptions,
+  body: (value: json.FromSchema<S>) => void,
+): void {
+  // Construct a base object that includes all required properties.
+  const required = makeFromSchema(false, schema);
+
+  // Identify optional properties.
+  const optionalKeys: Array<keyof S> = [];
+
+  for (const [key, validator] of Object.entries(schema)) {
+    if (!validator.required) {
+      optionalKeys.push(key);
+    }
+  }
+
+  const optionalValues = (key: keyof S) => [
+    null,
+    undefined,
+    `value-for-${String(key)}`,
+  ];
+
+  // Constructs an array of test objects, starting with `required` and adding
+  // acceptable values for any
+  const permutations = (keys: Array<keyof S>) => {
+    if (keys.length === 0) return [required];
+
+    const bases = permutations(keys.slice(1));
+    const result: Array<json.FromSchema<S>> = [];
+
+    const optionalKey = keys[0];
+    for (const base of bases) {
+      if (!opts.excludeAbsent) {
+        // Optional keys can be absent entirely.
+        result.push(base);
+      }
+
+      // Or be present and have one of the `optionalValues`.
+      for (const optionalValue of optionalValues(optionalKey)) {
+        result.push({ ...base, [optionalKey]: optionalValue });
+      }
+    }
+    return result;
+  };
+
+  // Call `body` for all test cases.
+  const testCases = permutations(optionalKeys);
+  for (const testCase of testCases) {
+    try {
+      body(testCase);
+    } catch (err) {
+      t.log(testCase);
+      throw err;
+    }
+  }
+}

src/json/testing-util.ts

@@ -532,7 +532,7 @@ test(
     t.is(results[0].type, "git_server");
     t.is(results[0].host, "https://github.com/");
 
-    if (startProxyExports.isUsernamePassword(results[0])) {
+    if (startProxyExports.hasUsernameAndPassword(results[0])) {
       t.assert(results[0].password?.startsWith("ghp_"));
     } else {
       t.fail("Expected a `UsernamePassword`-based credential.");
@@ -563,7 +563,7 @@ test(
     t.is(results[0].type, "git_server");
     t.is(results[0].host, "https://github.com/");
 
-    if (startProxyExports.isUsernamePassword(results[0])) {
+    if (startProxyExports.hasUsernameAndPassword(results[0])) {
       t.assert(results[0].password?.startsWith("ghp_"));
     } else {
       t.fail("Expected a `UsernamePassword`-based credential.");

src/start-proxy.test.ts

@@ -24,8 +24,8 @@ import {
   Address,
   Registry,
   Credential,
-  isToken,
-  isUsernamePassword,
+  hasToken,
+  hasUsernameAndPassword,
   hasUsername,
   RawCredential,
 } from "./start-proxy/types";
@@ -331,11 +331,11 @@ export function getCredentials(
     const noUsername =
       !hasUsername(authConfig) || !isDefined(authConfig.username);
     const passwordIsPAT =
-      isUsernamePassword(authConfig) &&
+      hasUsernameAndPassword(authConfig) &&
       isDefined(authConfig.password) &&
       isPAT(authConfig.password);
     const tokenIsPAT =
-      isToken(authConfig) &&
+      hasToken(authConfig) &&
       isDefined(authConfig.token) &&
       isPAT(authConfig.token);
 

src/start-proxy.ts

@@ -1,6 +1,6 @@
 import test from "ava";
 
-import { makeFromSchema } from "../json/testing-util";
+import { makeFromSchema, withSchemaMatrix } from "../json/testing-util";
 import { setupTests } from "../testing-utils";
 
 import * as types from "./types";
@@ -27,6 +27,38 @@ const validJFrogCredential: types.JFrogConfig = {
   "identity-mapping-name": "my-mapping",
 };
 
+test("hasUsername", (t) => {
+  // Reject the case where `username` is missing.
+  t.false(types.hasUsername({}));
+
+  // Test all cases where `username` is present.
+  withSchemaMatrix(
+    t,
+    types.usernameSchema,
+    { excludeAbsent: true },
+    (value) => {
+      t.true(types.hasUsername(value));
+    },
+  );
+});
+
+test("hasUsernameAndPassword", (t) => {
+  // Reject cases where `username` or `password` are missing.
+  t.false(types.hasUsernameAndPassword({}));
+  t.false(types.hasUsernameAndPassword({ username: "foo" }));
+  t.false(types.hasUsernameAndPassword({ password: "foo" }));
+
+  // Test all cases where both `username` and `password` are present.
+  withSchemaMatrix(
+    t,
+    types.usernamePasswordSchema,
+    { excludeAbsent: true },
+    (value) => {
+      t.true(types.hasUsernameAndPassword(value));
+    },
+  );
+});
+
 test("credentialToStr - pretty-prints valid username+password configurations", (t) => {
   const secret = "password123";
   const credential: types.Credential = {

src/start-proxy/types.test.ts

@@ -18,10 +18,11 @@ export const usernameSchema = {
 /** Usernames may be present for both authentication with tokens or passwords. */
 export type Username = json.FromSchema<typeof usernameSchema>;
 
-/** Decides whether `config` has a username. */
-export function hasUsername(
-  config: UnvalidatedObject<unknown>,
-): config is Username {
+/**
+ * Narrows `config` to `Username` if `config` has a `username` property.
+ * Not used for validation. Assumes that `config` is already a validated `AuthConfig`.
+ */
+export function hasUsername(config: AuthConfig): config is Username {
   return "username" in config;
 }
 
@@ -38,11 +39,14 @@ export const usernamePasswordSchema = {
  */
 export type UsernamePassword = json.FromSchema<typeof usernamePasswordSchema>;
 
-/** Decides whether `config` is based on a username and password. */
-export function isUsernamePassword(
+/**
+ * Narrows `config` to `UsernamePassword` if it has a `username` and `password` property.
+ * Not used for validation. Assumes that `config` is already a validated `AuthConfig`.
+ */
+export function hasUsernameAndPassword(
   config: AuthConfig,
 ): config is UsernamePassword {
-  return json.validateSchema(usernamePasswordSchema, config);
+  return hasUsername(config) && "password" in config;
 }
 
 /** A schema for credential objects for token-based authentication. */
@@ -58,6 +62,14 @@ export const tokenSchema = {
  */
 export type Token = json.FromSchema<typeof tokenSchema>;
 
+/**
+ * Narrows `config` to `Token` if it has a `token` property.
+ * Not used for validation. Assumes that `config` is already a validated `AuthConfig`.
+ */
+export function hasToken(config: AuthConfig): config is Token {
+  return "token" in config;
+}
+
 /** Decides whether `config` is token-based. */
 export function isToken(
   config: UnvalidatedObject<AuthConfig>,
@@ -205,7 +217,7 @@ export function credentialToStr(credential: Credential): string {
       isDefined(credential.password) ? "***" : undefined,
     );
   }
-  if (isToken(credential)) {
+  if (hasToken(credential)) {
     appendIfDefined("Token", isDefined(credential.token) ? "***" : undefined);
   }