bug: handle suffixes and commas in availableMemoryMb (#335)

* bug: handle suffixes and commas in availableMemoryMb

* add more test coverage

Author: sethvargo

README.md

@@ -75,7 +75,7 @@ jobs:
 
 - `entry_point`: (Optional) Name of a function (as defined in source code) that will be executed. Defaults to the resource name suffix, if not specified.
 
-- `memory_mb`: (Optional) The amount of memory in MB available for a function. Defaults to 256MB.
+- `memory_mb`: (Optional) The amount of memory in MB available for a function. Defaults to '256' (256 MB). The value must be the number of megabytes _without_ the unit suffix.
 
 - `region`: (Optional) [Region](https://cloud.google.com/functions/docs/locations) in which the function should be deployed. Defaults to `us-central1`.
 

action.yaml

@@ -80,7 +80,7 @@ inputs:
 
   memory_mb:
     description: |-
-      The amount of memory in MB available for a function. Defaults to 256MB.
+      The amount of memory in MB available for a function. Defaults to 256.
     required: false
 
   vpc_connector:

src/main.ts

@@ -39,7 +39,7 @@ import {
 
 import { CloudFunction, CloudFunctionsClient, SecretEnvVar, SecretVolume } from './client';
 import { SecretName } from './secret';
-import { formatEntry, toEnum } from './util';
+import { formatEntry, stringToInt, toEnum } from './util';
 
 async function run(): Promise<void> {
   try {
@@ -49,7 +49,7 @@ async function run(): Promise<void> {
     const description = presence(getInput('description'));
     const credentials = presence(getInput('credentials'));
     let projectID = presence(getInput('project_id'));
-    const availableMemoryMb = presence(getInput('memory_mb'));
+    const availableMemoryMb = stringToInt(getInput('memory_mb'));
     const region = presence(getInput('region') || 'us-central1');
     const envVars = presence(getInput('env_vars'));
     const envVarsFile = presence(getInput('env_vars_file'));
@@ -189,7 +189,7 @@ async function run(): Promise<void> {
       name: name,
       runtime: runtime,
       description: description,
-      availableMemoryMb: availableMemoryMb ? +availableMemoryMb : undefined,
+      availableMemoryMb: availableMemoryMb,
       buildEnvironmentVariables: buildEnvironmentVariables,
       buildWorkerPool: buildWorkerPool,
       dockerRegistry: dockerRegistry,

src/util.ts

@@ -118,3 +118,25 @@ export function formatEntry(entry: RealEntryData): string {
 export function toEnum(s: string): string {
   return (s || '').replace(/[\s-]+/g, '_').toUpperCase();
 }
+
+/**
+ * stringToInt is a helper that converts the given string into an integer. If
+ * the given string is empty, it returns undefined. If the string is not empty
+ * and parseInt fails (returns NaN), it throws an error. Otherwise, it returns
+ * the integer value.
+ *
+ * @param str String to parse as an int.
+ * @returns Parsed integer or undefined if the input was the empty string.
+ */
+export function stringToInt(str: string): number | undefined {
+  str = (str || '').trim().replace(/[_,]/g, '');
+  if (str === '') {
+    return undefined;
+  }
+
+  const result = parseInt(str);
+  if (isNaN(result)) {
+    throw new Error(`input "${str}" is not a number`);
+  }
+  return result;
+}

tests/client.test.ts

@@ -314,9 +314,9 @@ describe('CloudFunctionsClient', () => {
 
     cases.forEach((tc) => {
       it(tc.name, async () => {
-        if (tc.expected) {
+        if (tc.expected !== undefined) {
           expect(tc.client.fullResourceName(tc.input)).to.eql(tc.expected);
-        } else if (tc.error) {
+        } else if (tc.error !== undefined) {
           expect(() => {
             tc.client.fullResourceName(tc.input);
           }).to.throw(tc.error);

tests/secret.test.ts

@@ -88,12 +88,12 @@ describe('SecretName', function () {
 
   cases.forEach((tc) => {
     it(tc.name, async () => {
-      if (tc.expected) {
+      if (tc.expected !== undefined) {
         const secret = new SecretName(tc.input);
         expect(secret.project).to.eq(tc.expected.project);
         expect(secret.name).to.eq(tc.expected.secret);
         expect(secret.version).to.eq(tc.expected.version);
-      } else if (tc.error) {
+      } else if (tc.error !== undefined) {
         expect(() => {
           new SecretName(tc.input);
         }).to.throw(tc.error);

tests/util.test.ts

@@ -6,7 +6,7 @@ import { expect } from 'chai';
 import StreamZip from 'node-stream-zip';
 import { randomFilepath } from '@google-github-actions/actions-utils';
 
-import { toEnum, zipDir } from '../src/util';
+import { stringToInt, toEnum, zipDir } from '../src/util';
 
 describe('Util', () => {
   describe('#zipDir', () => {
@@ -44,11 +44,11 @@ describe('Util', () => {
 
     cases.forEach((tc) => {
       it(tc.name, async () => {
-        if (tc.expectedFiles) {
+        if (tc.expectedFiles !== undefined) {
           const zf = await zipDir(tc.zipDir, randomFilepath());
           const filesInsideZip = await getFilesInZip(zf);
           expect(filesInsideZip).to.have.members(tc.expectedFiles);
-        } else if (tc.error) {
+        } else if (tc.error !== undefined) {
           try {
             await zipDir(tc.zipDir, randomFilepath());
             throw new Error(`Should have thrown err: ${tc.error}`);
@@ -105,6 +105,65 @@ describe('Util', () => {
       });
     });
   });
+
+  describe('#stringToInt', () => {
+    const cases: {
+      only?: boolean;
+      name: string;
+      input: string;
+      expected?: number | undefined;
+      error?: string;
+    }[] = [
+      {
+        name: 'empty',
+        input: '',
+        expected: undefined,
+      },
+      {
+        name: 'spaces',
+        input: ' ',
+        expected: undefined,
+      },
+      {
+        name: 'digit',
+        input: '1',
+        expected: 1,
+      },
+      {
+        name: 'multi-digit',
+        input: '123',
+        expected: 123,
+      },
+      {
+        name: 'suffix',
+        input: '100MB',
+        expected: 100,
+      },
+      {
+        name: 'comma',
+        input: '1,000',
+        expected: 1000,
+      },
+      {
+        name: 'NaN',
+        input: 'this is definitely not a number',
+        error: 'input "this is definitely not a number" is not a number',
+      },
+    ];
+
+    cases.forEach((tc) => {
+      const fn = tc.only ? it.only : it;
+      fn(tc.name, () => {
+        if (tc.expected !== undefined) {
+          expect(stringToInt(tc.input)).to.eql(tc.expected);
+        } else if (tc.error !== undefined) {
+          expect(() => {
+            stringToInt(tc.input);
+          }).to.throw(tc.error);
+        }
+      });
+    });
+  });
 });
 
 /**