Repository: serverless/serverless-azure-functions
Pull request #290 (2019-08-29 23:07:14)

fix: Combine rollback log statement into one string
Strings were being logged out of order at random for some reason. Seemed cleaner anyway to just combine them into one string and adjusted tests.


Patch:
diff --git a/package-lock.json b/package-lock.json
index 0ea29ab..ec97466 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,6 +1,6 @@
 {
   "name": "serverless-azure-functions",
-  "version": "1.0.0-25",
+  "version": "1.0.0-31",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {
@@ -5164,8 +5164,7 @@
         },
         "console-control-strings": {
           "version": "1.1.0",
-          "bundled": true,
-          "optional": true
+          "bundled": true
         },
         "core-util-is": {
           "version": "1.0.2",
@@ -5268,8 +5267,7 @@
         },
         "inherits": {
           "version": "2.0.3",
-          "bundled": true,
-          "optional": true
+          "bundled": true
         },
         "ini": {
           "version": "1.3.5",
@@ -5303,7 +5301,6 @@
         "minipass": {
           "version": "2.3.5",
           "bundled": true,
-          "optional": true,
           "requires": {
             "safe-buffer": "^5.1.2",
             "yallist": "^3.0.0"
@@ -5320,7 +5317,6 @@
         "mkdirp": {
           "version": "0.5.1",
           "bundled": true,
-          "optional": true,
           "requires": {
             "minimist": "0.0.8"
           }
@@ -5393,7 +5389,8 @@
         },
         "number-is-nan": {
           "version": "1.0.1",
-          "bundled": true
+          "bundled": true,
+          "optional": true
         },
         "object-assign": {
           "version": "4.1.1",
@@ -5403,7 +5400,6 @@
         "once": {
           "version": "1.4.0",
           "bundled": true,
-          "optional": true,
           "requires": {
             "wrappy": "1"
           }
@@ -5479,8 +5475,7 @@
         },
         "safe-buffer": {
           "version": "5.1.2",
-          "bundled": true,
-          "optional": true
+          "bundled": true
         },
         "safer-buffer": {
           "version": "2.1.2",
@@ -5510,7 +5505,6 @@
         "string-width": {
           "version": "1.0.2",
           "bundled": true,
-          "optional": true,
           "requires": {
             "code-point-at": "^1.0.0",
             "is-fullwidth-code-point": "^1.0.0",
@@ -5567,13 +5561,11 @@
         },
         "wrappy": {
           "version": "1.0.2",
-          "bundled": true,
-          "optional": true
+          "bundled": true
         },
         "yallist": {
           "version": "3.0.3",
-          "bundled": true,
-          "optional": true
+          "bundled": true
         }
       }
     },
diff --git a/package.json b/package.json
index d1a0d55..62415ef 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "serverless-azure-functions",
-  "version": "1.0.0-25",
+  "version": "1.0.0-31",
   "description": "Provider plugin for the Serverless Framework v1.x which adds support for Azure Functions.",
   "license": "MIT",
   "main": "./lib/index.js",
diff --git a/src/armTemplates/resources/appServicePlan.ts b/src/armTemplates/resources/appServicePlan.ts
index 50d99e6..d38531e 100644
--- a/src/armTemplates/resources/appServicePlan.ts
+++ b/src/armTemplates/resources/appServicePlan.ts
@@ -4,7 +4,7 @@ import { AzureNamingService } from "../../services/namingService";
 
 export class AppServicePlanResource implements ArmResourceTemplateGenerator {
   public static getResourceName(config: ServerlessAzureConfig) {
-    return AzureNamingService.getResourceName(config, config.provider.appInsights, "asp");
+    return AzureNamingService.getResourceName(config, config.provider.appServicePlan, "asp");
   }
 
   public getTemplate(): ArmResourceTemplate {
diff --git a/src/armTemplates/resources/functionApp.ts b/src/armTemplates/resources/functionApp.ts
index eab4a9e..3b281ac 100644
--- a/src/armTemplates/resources/functionApp.ts
+++ b/src/armTemplates/resources/functionApp.ts
@@ -10,7 +10,7 @@ export class FunctionAppResource implements ArmResourceTemplateGenerator {
   public static getResourceName(config: ServerlessAzureConfig) {
     const safeServiceName = config.service.replace(/\s/g, "-");
 
-    return AzureNamingService.getResourceName(config, config.provider.appInsights, safeServiceName);
+    return AzureNamingService.getResourceName(config, config.provider.functionApp, safeServiceName);
   }
 
   public getTemplate(): ArmResourceTemplate {
@@ -123,26 +123,26 @@ export class FunctionAppResource implements ArmResourceTemplateGenerator {
     };
   }
 
-  private getRuntimeVersion(runtime: string): string{ 
-    if(!runtime) {
+  private getRuntimeVersion(runtime: string): string {
+    if (!runtime) {
       throw new Error("Runtime version not specified in serverless.yml");
     }
     const extractedVersion = runtime.split("nodejs")[1];
     const runtimeVersionsList = runtimeVersionsJson["nodejs"];
 
     //Searches for a specific version. For example nodejs10.6.0.
-    if(!extractedVersion.endsWith(".x")) {
+    if (!extractedVersion.endsWith(".x")) {
       let retrivedVersion: string;
-      for(const version of runtimeVersionsList){
-        retrivedVersion = version["version"]; 
-        if(extractedVersion === retrivedVersion && semver.valid(retrivedVersion)){
-          return retrivedVersion; 
+      for (const version of runtimeVersionsList) {
+        retrivedVersion = version["version"];
+        if (extractedVersion === retrivedVersion && semver.valid(retrivedVersion)) {
+          return retrivedVersion;
         }
       }
     }
     else {
       // User specified something like nodejs10.14.x
-      const extractedVersionNumber = extractedVersion.replace(/[^0-9\.]/g,"");
+      const extractedVersionNumber = extractedVersion.replace(/[^0-9\.]/g, "");
 
       const selectedVersions = runtimeVersionsList.filter(({ version }) => {
         return version.startsWith(extractedVersionNumber) && semver.valid(version)
@@ -154,5 +154,5 @@ export class FunctionAppResource implements ArmResourceTemplateGenerator {
       return selectedVersions.sort(semver.rcompare)[0]
     }
     throw new Error(`Could not find runtime version matching ${runtime}`)
-  }  
+  }
 }
diff --git a/src/models/serverless.ts b/src/models/serverless.ts
index 517e7f1..cfc161a 100644
--- a/src/models/serverless.ts
+++ b/src/models/serverless.ts
@@ -36,6 +36,7 @@ export interface ServerlessAzureProvider {
   region: string;
   stage: string;
   name: string;
+  subscriptionId?: string;
   environment?: {
     [key: string]: any;
   };
diff --git a/src/plugins/login/azureLoginPlugin.ts b/src/plugins/login/azureLoginPlugin.ts
index e3f9692..137b387 100644
--- a/src/plugins/login/azureLoginPlugin.ts
+++ b/src/plugins/login/azureLoginPlugin.ts
@@ -23,12 +23,20 @@ export class AzureLoginPlugin extends AzureBasePlugin<AzureLoginOptions> {
     this.log("Logging into Azure");
 
     try {
-      const authResult = await AzureLoginService.login();
+      const loginService = new AzureLoginService(this.serverless, this.options);
+      const authResult = await loginService.login();
+
       this.serverless.variables["azureCredentials"] = authResult.credentials;
       // Use environment variable for sub ID or use the first subscription in the list (service principal can
       // have access to more than one subscription)
-      this.serverless.variables["subscriptionId"] = this.options.subscriptionId || process.env.azureSubId || this.serverless.service.provider["subscriptionId"] || authResult.subscriptions[0].id;
-      this.serverless.cli.log(`Using subscription ID: ${this.serverless.variables["subscriptionId"]}`);
+      let subId = loginService.getSubscriptionId();
+      if (!subId && !authResult.subscriptions.length) {
+        throw new Error("Authentication returned an empty list of subscriptions. " +
+          "Try another form of authentication. See the serverless-azure-functions README for more help");
+      }
+      subId = subId || authResult.subscriptions[0].id;
+      this.serverless.variables["subscriptionId"] = subId;
+      this.serverless.cli.log(`Using subscription ID: ${subId}`);
     }
     catch (e) {
       this.log("Error logging into azure");
diff --git a/src/services/azureBlobStorageService.ts b/src/services/azureBlobStorageService.ts
index ba04917..144e7bf 100644
--- a/src/services/azureBlobStorageService.ts
+++ b/src/services/azureBlobStorageService.ts
@@ -45,7 +45,7 @@ export class AzureBlobStorageService extends BaseService {
     if (this.storageCredential) {
       return;
     }
-    this.storageCredential = (this.authType === AzureStorageAuthType.SharedKey) 
+    this.storageCredential = (this.authType === AzureStorageAuthType.SharedKey)
       ?
       new SharedKeyCredential(this.storageAccountName, await this.getKey())
       :
@@ -91,10 +91,10 @@ export class AzureBlobStorageService extends BaseService {
         blockSize: 4 * 1024 * 1024, // 4MB block size
         parallelism: 20, // 20 concurrency
       }
-    );    
+    );
     fs.writeFileSync(targetPath, buffer, "binary");
   }
-  
+
   /**
    * Delete a blob from Azure Blob Storage
    * @param containerName Name of container containing blob
@@ -195,7 +195,7 @@ export class AzureBlobStorageService extends BaseService {
   public async generateBlobSasTokenUrl(containerName: string, blobName: string, days: number = 365): Promise<string> {
     this.checkInitialization();
     if (this.authType !== AzureStorageAuthType.SharedKey) {
-      throw new Error("Need to authenticate with shared key in order to generate SAS tokens. " + 
+      throw new Error("Need to authenticate with shared key in order to generate SAS tokens. " +
       "Initialize Blob Service with SharedKey auth type");
     }
 
@@ -219,7 +219,7 @@ export class AzureBlobStorageService extends BaseService {
       version: "2016-05-31"
     },
     this.storageCredential as SharedKeyCredential);
-    
+
     const blobUrl = this.getBlockBlobURL(containerName, blobName);
     return `${blobUrl.url}?${blobSas}`
   }
@@ -274,7 +274,8 @@ export class AzureBlobStorageService extends BaseService {
    * Get access token by logging in (again) with a storage-specific context
    */
   private async getToken(): Promise<string> {
-    const authResponse = await AzureLoginService.login({
+    const loginService = new AzureLoginService(this.serverless, this.options);
+    const authResponse = await loginService.login({
       tokenAudience: "https://storage.azure.com/"
     });
     const token = await authResponse.credentials.getToken();
@@ -296,8 +297,8 @@ export class AzureBlobStorageService extends BaseService {
    * the credentials will not be available for any operation
    */
   private checkInitialization() {
-    Guard.null(this.storageCredential, "storageCredential", 
-      "Azure Blob Storage Service has not been initialized. Make sure .initialize() has been called " + 
+    Guard.null(this.storageCredential, "storageCredential",
+      "Azure Blob Storage Service has not been initialized. Make sure .initialize() has been called " +
       "before performing any operation");
   }
 }
diff --git a/src/services/baseService.ts b/src/services/baseService.ts
index f6797e2..e2da323 100644
--- a/src/services/baseService.ts
+++ b/src/services/baseService.ts
@@ -41,7 +41,7 @@ export abstract class BaseService {
     this.baseUrl = "https://management.azure.com";
     this.serviceName = this.getServiceName();
     this.credentials = serverless.variables["azureCredentials"];
-    this.subscriptionId = serverless.variables["subscriptionId"];
+    this.subscriptionId = this.config.provider.subscriptionId;
     this.resourceGroup = this.getResourceGroupName();
     this.deploymentConfig = this.getDeploymentConfig();
     this.deploymentName = this.getDeploymentName();
@@ -83,6 +83,13 @@ export abstract class BaseService {
     return this.config.provider.resourceGroup;
   }
 
+  /**
+   * Azure Subscription ID
+   */
+  public getSubscriptionId(): string {
+    return this.config.provider.subscriptionId;
+  }
+
   /**
    * Deployment config from `serverless.yml` or default.
    * Defaults can be found in the `config.ts` file
@@ -250,12 +257,16 @@ export abstract class BaseService {
    * in config if passed through CLI
    */
   private setConfigFromCli() {
-    const { prefix, region, stage } = this.config.provider;
+    const { prefix, region, stage, subscriptionId } = this.config.provider;
     this.config.provider = {
       ...this.config.provider,
       prefix: this.getOption("prefix") || prefix,
       stage: this.getOption("stage") || stage,
       region: this.getOption("region") || region,
+      subscriptionId: this.getOption("subscriptionId")
+        || process.env.azureSubId
+        || subscriptionId
+        || this.serverless.variables["subscriptionId"]
     }
     if (!this.config.provider.region && this.config.provider["location"]) {
       this.config.provider.region = this.config.provider["location"];
diff --git a/src/services/functionAppService.ts b/src/services/functionAppService.ts
index 353a658..06af8b2 100644
--- a/src/services/functionAppService.ts
+++ b/src/services/functionAppService.ts
@@ -15,7 +15,7 @@ import configConstants from "../config";
 
 export class FunctionAppService extends BaseService {
   private static readonly retryCount: number = 30;
-  private static readonly retryInterval: number = 5000;
+  private static readonly retryInterval: number = 30000;
   private webClient: WebSiteManagementClient;
   private blobService: AzureBlobStorageService;
 
@@ -94,13 +94,18 @@ export class FunctionAppService extends BaseService {
     Guard.null(functionApp);
 
     const getTokenUrl = `${this.baseUrl}${functionApp.id}/functions?api-version=2016-08-01`;
+    let retries = 0;
     try {
       const response = await Utils.runWithRetry(async () => {
         const listFunctionsResponse = await this.sendApiRequest("GET", getTokenUrl);
 
         if (listFunctionsResponse.status !== 200 || listFunctionsResponse.data.value.length === 0) {
-          this.log("-> Function App not ready. Retrying...");
-          throw new Error(JSON.stringify(listFunctionsResponse.data, null, 2));
+          this.log(`-> Function App not ready. Retry ${retries++} of ${FunctionAppService.retryCount}...`);
+          const response = JSON.stringify(listFunctionsResponse.data, null, 2);
+          throw new Error(
+            `The function app is taking longer than usual to be provisioned. Please try again soon.
+            Response error data: \n${response}`
+          );
         }
 
         return listFunctionsResponse;
diff --git a/src/services/loginService.ts b/src/services/loginService.ts
index 6bf433f..8ed84dc 100644
--- a/src/services/loginService.ts
+++ b/src/services/loginService.ts
@@ -6,34 +6,40 @@ import {
   AuthResponse,
   AzureTokenCredentialsOptions,
   InteractiveLoginOptions,
-  DeviceTokenCredentials,  
+  DeviceTokenCredentials,
 } from "@azure/ms-rest-nodeauth";
 import { SimpleFileTokenCache } from "../plugins/login/utils/simpleFileTokenCache";
+import { BaseService } from "./baseService";
 
 export interface AzureLoginOptions extends Serverless.Options {
   subscriptionId?: string;
 }
 
-export class AzureLoginService {
+export class AzureLoginService extends BaseService {
+
+  public constructor(serverless: Serverless, options: Serverless.Options) {
+    super(serverless, options, false);
+  }
+
   /**
    * Logs in via service principal login if environment variables are
    * set or via interactive login if environment variables are not set
    * @param options Options for different authentication methods
    */
-  public static async login(options?: AzureTokenCredentialsOptions|InteractiveLoginOptions): Promise<AuthResponse> {
+  public async login(options?: AzureTokenCredentialsOptions|InteractiveLoginOptions): Promise<AuthResponse> {
     const subscriptionId = process.env.azureSubId;
     const clientId = process.env.azureServicePrincipalClientId;
     const secret = process.env.azureServicePrincipalPassword;
     const tenantId = process.env.azureServicePrincipalTenantId;
 
     if (subscriptionId && clientId && secret && tenantId) {
-      return await AzureLoginService.servicePrincipalLogin(clientId, secret, tenantId, options);
+      return await this.servicePrincipalLogin(clientId, secret, tenantId, options);
     } else {
-      return await AzureLoginService.interactiveLogin(options);
+      return await this.interactiveLogin(options);
     }
   }
 
-  public static async interactiveLogin(options?: InteractiveLoginOptions): Promise<AuthResponse> {
+  public async interactiveLogin(options?: InteractiveLoginOptions): Promise<AuthResponse> {
     let authResp: AuthResponse = {credentials: undefined, subscriptions: []};
     const fileTokenCache = new SimpleFileTokenCache();
     if(fileTokenCache.isEmpty()){
@@ -49,7 +55,7 @@ export class AzureLoginService {
 
   }
 
-  public static async servicePrincipalLogin(clientId: string, secret: string, tenantId: string, options: AzureTokenCredentialsOptions): Promise<AuthResponse> {
+  public async servicePrincipalLogin(clientId: string, secret: string, tenantId: string, options: AzureTokenCredentialsOptions): Promise<AuthResponse> {
     return loginWithServicePrincipalSecretWithAuthResponse(clientId, secret, tenantId, options);
   }
 }
diff --git a/src/services/offlineService.ts b/src/services/offlineService.ts
index 48da80e..e085c63 100644
--- a/src/services/offlineService.ts
+++ b/src/services/offlineService.ts
@@ -13,7 +13,7 @@ export class OfflineService extends BaseService {
     "local.settings.json": JSON.stringify({
       IsEncrypted: false,
       Values: {
-        AzureWebJobsStorage: "",
+        AzureWebJobsStorage: "UseDevelopmentStorage=true",
         FUNCTIONS_WORKER_RUNTIME: "node"
       }
     }),
diff --git a/src/services/rollbackService.ts b/src/services/rollbackService.ts
index a857693..540086d 100644
--- a/src/services/rollbackService.ts
+++ b/src/services/rollbackService.ts
@@ -8,6 +8,7 @@ import { BaseService } from "./baseService";
 import { FunctionAppService } from "./functionAppService";
 import { ResourceService } from "./resourceService";
 import { ArmDeployment } from "../models/armTemplates";
+import fs from "fs";
 
 /**
  * Services for the Rollback Plugin
@@ -98,6 +99,9 @@ export class RollbackService extends BaseService {
     const functionAppService = new FunctionAppService(this.serverless, this.options);
     const functionApp = await functionAppService.get();
     await functionAppService.uploadZippedArfifactToFunctionApp(functionApp, artifactPath);
+    if (fs.existsSync(artifactPath)) {
+      fs.unlinkSync(artifactPath);
+    }
   }
 
   /**
@@ -107,10 +111,10 @@ export class RollbackService extends BaseService {
   private async getDeployment(): Promise<DeploymentExtended> {
     let timestamp = Utils.get(this.options, "timestamp");
     if (!timestamp) {
-      this.log("Need to specify a timestamp for rollback.");
-      this.log("Example usage:\n\nsls rollback -t 1562014362");
-      this.log(await this.resourceService.listDeployments());
-      return null;
+      this.log("Need to specify a timestamp for rollback.\n\n" +
+        "Example usage:\n\nsls rollback -t 1562014362\n\n" +
+        await this.resourceService.listDeployments());
+      return;
     }
     const deployments = await this.getArmDeploymentsByTimestamp();
     const deployment = deployments.get(timestamp);


Test patch:
diff --git a/pipelines/templates/unit-tests.yml b/pipelines/templates/unit-tests.yml
index 452862a..50cd3d3 100644
--- a/pipelines/templates/unit-tests.yml
+++ b/pipelines/templates/unit-tests.yml
@@ -19,6 +19,8 @@ jobs:
       versionSpec: ${{ parameters.nodeVersion }}.x
 
   - bash: |
+      set -e
+      
       npm ci
       npm run compile
       npm run test:coverage  # linting is run as part of pretest
diff --git a/src/plugins/login/azureLoginPlugin.test.ts b/src/plugins/login/azureLoginPlugin.test.ts
index 89c8a13..292b1c4 100644
--- a/src/plugins/login/azureLoginPlugin.test.ts
+++ b/src/plugins/login/azureLoginPlugin.test.ts
@@ -4,10 +4,10 @@ import { MockFactory } from "../../test/mockFactory";
 import { invokeHook, setEnvVariables, unsetEnvVariables } from "../../test/utils";
 import { AzureLoginPlugin } from "./azureLoginPlugin";
 import { loginHooks } from "./loginHooks";
+import { ServerlessAzureConfig } from "../../models/serverless";
 
 describe("Login Plugin", () => {
 
-  const authResponse = MockFactory.createTestAuthResponse();
   const envVariables = MockFactory.createTestServicePrincipalEnvVariables()
   const credentials = MockFactory.createTestVariables().azureCredentials;
 
@@ -19,8 +19,8 @@ describe("Login Plugin", () => {
     return new AzureLoginPlugin(sls, options || MockFactory.createTestServerlessOptions());
   }
 
-  function createMockLoginFunction() {
-    return jest.fn(() => Promise.resolve(authResponse));
+  function createMockLoginFunction(authResponse?) {
+    return jest.fn(() => Promise.resolve(authResponse || MockFactory.createTestAuthResponse()));
   }
 
   function setServicePrincipalEnvVariables() {
@@ -37,8 +37,8 @@ describe("Login Plugin", () => {
   }
 
   beforeEach(() => {
-    AzureLoginService.interactiveLogin = createMockLoginFunction();
-    AzureLoginService.servicePrincipalLogin = createMockLoginFunction();
+    AzureLoginService.prototype.interactiveLogin = createMockLoginFunction();
+    AzureLoginService.prototype.servicePrincipalLogin = createMockLoginFunction();
   });
 
   it("contains the hooks as contained in loginHooks", () => {
@@ -47,28 +47,28 @@ describe("Login Plugin", () => {
 
   it("returns if azure credentials are set", async () => {
     await invokeLoginHook(true);
-    expect(AzureLoginService.interactiveLogin).not.toBeCalled();
-    expect(AzureLoginService.servicePrincipalLogin).not.toBeCalled();
+    expect(AzureLoginService.prototype.interactiveLogin).not.toBeCalled();
+    expect(AzureLoginService.prototype.servicePrincipalLogin).not.toBeCalled();
   });
 
   it("calls login if azure credentials are not set", async () => {
     unsetServicePrincipalEnvVariables();
     await invokeLoginHook();
-    expect(AzureLoginService.interactiveLogin).toBeCalled();
-    expect(AzureLoginService.servicePrincipalLogin).not.toBeCalled();
+    expect(AzureLoginService.prototype.interactiveLogin).toBeCalled();
+    expect(AzureLoginService.prototype.servicePrincipalLogin).not.toBeCalled();
   });
 
   it("calls service principal login if environment variables are set", async () => {
     setServicePrincipalEnvVariables();
     const sls = MockFactory.createTestServerless();
     await invokeLoginHook(false, sls);
-    expect(AzureLoginService.servicePrincipalLogin).toBeCalledWith(
+    expect(AzureLoginService.prototype.servicePrincipalLogin).toBeCalledWith(
       "azureServicePrincipalClientId",
       "azureServicePrincipalPassword",
       "azureServicePrincipalTenantId",
       undefined // would be options
     )
-    expect(AzureLoginService.interactiveLogin).not.toBeCalled();
+    expect(AzureLoginService.prototype.interactiveLogin).not.toBeCalled();
     expect(JSON.stringify(sls.variables["azureCredentials"])).toEqual(JSON.stringify(credentials));
     expect(sls.variables["subscriptionId"]).toEqual("azureSubId");
   });
@@ -77,8 +77,8 @@ describe("Login Plugin", () => {
     unsetServicePrincipalEnvVariables();
     const sls = MockFactory.createTestServerless();
     await invokeLoginHook(false, sls);
-    expect(AzureLoginService.servicePrincipalLogin).not.toBeCalled();
-    expect(AzureLoginService.interactiveLogin).toBeCalled();
+    expect(AzureLoginService.prototype.servicePrincipalLogin).not.toBeCalled();
+    expect(AzureLoginService.prototype.interactiveLogin).toBeCalled();
     expect(JSON.stringify(sls.variables["azureCredentials"])).toEqual(JSON.stringify(credentials));
     expect(sls.variables["subscriptionId"]).toEqual("azureSubId");
   });
@@ -86,42 +86,48 @@ describe("Login Plugin", () => {
   it("logs an error from authentication and crashes with it", async () => {
     unsetServicePrincipalEnvVariables();
     const error = new Error("This is my error message")
-    AzureLoginService.interactiveLogin = jest.fn(() => {
+    AzureLoginService.prototype.interactiveLogin = jest.fn(() => {
       throw error;
     });
     const sls = MockFactory.createTestServerless();
     await expect(invokeLoginHook(false, sls)).rejects.toThrow(error);
-    expect(AzureLoginService.interactiveLogin).toBeCalled()
-    expect(AzureLoginService.servicePrincipalLogin).not.toBeCalled();
+    expect(AzureLoginService.prototype.interactiveLogin).toBeCalled();
+    expect(AzureLoginService.prototype.servicePrincipalLogin).not.toBeCalled();
     expect(sls.cli.log).lastCalledWith("Error logging into azure");
   });
 
-  it("Uses the user specified subscription ID", async () => {
+  it("Uses the default subscription ID" , async () => {
     const sls = MockFactory.createTestServerless();
     const opt = MockFactory.createTestServerlessOptions();
-    opt["subscriptionId"] = "test-subs-id";
     await invokeLoginHook(false, sls, opt);
-    expect(AzureLoginService.interactiveLogin).toBeCalled()
-    expect(sls.variables["subscriptionId"]).toEqual("test-subs-id");
-    expect(sls.cli.log).toBeCalledWith("Using subscription ID: test-subs-id");
+    expect(AzureLoginService.prototype.interactiveLogin).toBeCalled();
+    expect(sls.variables["subscriptionId"]).toEqual("azureSubId");
+    expect(sls.cli.log).toBeCalledWith("Using subscription ID: azureSubId");
   });
 
-  it("Uses the default subscription ID" , async () => {
+  it("Throws an error with empty subscription list", async () => {
+    unsetServicePrincipalEnvVariables();
+    const authResponse = MockFactory.createTestAuthResponse();
+    authResponse.subscriptions = [];
+    AzureLoginService.prototype.interactiveLogin = createMockLoginFunction(authResponse);
     const sls = MockFactory.createTestServerless();
+    delete sls.variables["subscriptionId"];
     const opt = MockFactory.createTestServerlessOptions();
-    await invokeLoginHook(false, sls, opt);
-    expect(AzureLoginService.interactiveLogin).toBeCalled()
-    expect(sls.variables["subscriptionId"]).toEqual("azureSubId");
-    expect(sls.cli.log).toBeCalledWith("Using subscription ID: azureSubId");
+    await expect(invokeLoginHook(false, sls, opt)).rejects.toThrow();
+    expect(AzureLoginService.prototype.interactiveLogin).toBeCalled();
   });
 
-  it("Uses the subscription ID specified in serverless yaml", async () => {
+  it("Does not throw an error with empty subscription list if subscription previously specified", async () => {
+    unsetServicePrincipalEnvVariables();
+    const authResponse = MockFactory.createTestAuthResponse();
+    authResponse.subscriptions = [];
+    AzureLoginService.prototype.interactiveLogin = createMockLoginFunction(authResponse);
     const sls = MockFactory.createTestServerless();
+    delete sls.variables["subscriptionId"];
+    const subId = "my subscription id";
+    (sls.service as any as ServerlessAzureConfig).provider.subscriptionId = subId;
     const opt = MockFactory.createTestServerlessOptions();
-    sls.service.provider["subscriptionId"] = "test-subs-id";
-    await invokeLoginHook(false, sls, opt);
-    expect(AzureLoginService.interactiveLogin).toBeCalled()
-    expect(sls.variables["subscriptionId"]).toEqual("test-subs-id");
-    expect(sls.cli.log).toBeCalledWith("Using subscription ID: test-subs-id");
+    await invokeLoginHook(false, sls, opt)
+    expect(AzureLoginService.prototype.interactiveLogin).toBeCalled();
   });
 });
diff --git a/src/services/azureBlobStorageService.test.ts b/src/services/azureBlobStorageService.test.ts
index 6eb6e62..1d84f65 100644
--- a/src/services/azureBlobStorageService.test.ts
+++ b/src/services/azureBlobStorageService.test.ts
@@ -49,7 +49,7 @@ describe("Azure Blob Storage Service", () => {
     }) as any;
 
     BlockBlobURL.fromContainerURL = jest.fn(() => blockBlobUrl) as any;
-    AzureLoginService.login = jest.fn(() => Promise.resolve({
+    AzureLoginService.prototype.login = jest.fn(() => Promise.resolve({
       credentials: {
         getToken: jest.fn(() => {
           return {
@@ -142,7 +142,7 @@ describe("Azure Blob Storage Service", () => {
     expect(ContainerURL.fromServiceURL).not.toBeCalled();
     expect(ContainerURL.prototype.create).not.toBeCalled
   })
-  
+
   it("should delete a container", async () => {
     const containerToDelete = "delete container";
     ContainerURL.fromServiceURL = jest.fn(() => new ContainerURL(null, null));
diff --git a/src/services/baseService.test.ts b/src/services/baseService.test.ts
index 244eb9b..af6c23f 100644
--- a/src/services/baseService.test.ts
+++ b/src/services/baseService.test.ts
@@ -39,8 +39,11 @@ class MockService extends BaseService {
 
 describe("Base Service", () => {
   let service: MockService;
-  let sls: Serverless;
+  let serverless: Serverless;
   const serviceName = "my-custom-service"
+  const loginResultSubscriptionId = "ABC123";
+  const envVarSubscriptionId = "env var sub id";
+
 
   const slsConfig = {
     service: serviceName,
@@ -54,24 +57,24 @@ describe("Base Service", () => {
     mockFs.restore();
   });
 
-  function createMockService(options?: Serverless.Options) {
+  function createMockService(sls: Serverless, options?: Serverless.Options) {
     sls.variables["azureCredentials"] = MockFactory.createTestAzureCredentials();
-    sls.variables["subscriptionId"] = "ABC123";
+    sls.variables["subscriptionId"] = loginResultSubscriptionId;
     Object.assign(sls.service, slsConfig);
 
     return new MockService(sls, options);
   }
 
   beforeEach(() => {
-    sls = MockFactory.createTestServerless();
-    service = createMockService();
+    serverless = MockFactory.createTestServerless();
+    service = createMockService(serverless);
   });
 
   it("Initializes common service properties", () => {
     const props = service.getProperties();
     expect(props.baseUrl).toEqual("https://management.azure.com");
-    expect(props.credentials).toEqual(sls.variables["azureCredentials"]);
-    expect(props.subscriptionId).toEqual(sls.variables["subscriptionId"]);
+    expect(props.credentials).toEqual(serverless.variables["azureCredentials"]);
+    expect(props.subscriptionId).toEqual(serverless.variables["subscriptionId"]);
     expect(props.serviceName).toEqual(slsConfig.service);
     expect(props.resourceGroup).toEqual(slsConfig.provider.resourceGroup);
     const expectedDeploymentNameRegex = new RegExp(slsConfig.provider.deploymentName + "-t([0-9]+)")
@@ -79,11 +82,11 @@ describe("Base Service", () => {
   });
 
   it("Sets default region and stage values if not defined", () => {
-    const mockService = new MockService(sls);
+    const mockService = new MockService(serverless);
 
     expect(mockService).not.toBeNull();
-    expect(sls.service.provider.region).toEqual("westus");
-    expect(sls.service.provider.stage).toEqual("dev");
+    expect(serverless.service.provider.region).toEqual("westus");
+    expect(serverless.service.provider.stage).toEqual("dev");
   });
 
   it("returns region and stage based on CLI options", () => {
@@ -91,7 +94,7 @@ describe("Base Service", () => {
       stage: "prod",
       region: "eastus2",
     };
-    const mockService = new MockService(sls, cliOptions);
+    const mockService = new MockService(serverless, cliOptions);
 
     expect(mockService.getRegion()).toEqual(cliOptions.region);
     expect(mockService.getStage()).toEqual(cliOptions.stage);
@@ -105,48 +108,48 @@ describe("Base Service", () => {
       resourceGroup: resourceGroupName
     };
 
-    const mockService = new MockService(sls, cliOptions);
+    const mockService = new MockService(serverless, cliOptions);
     const actualResourceGroupName = mockService.getResourceGroupName();
 
     expect(actualResourceGroupName).toEqual(resourceGroupName);
   });
 
   it("use the resource group name from sls yaml config", () => {
-    const mockService = new MockService(sls);
+    const mockService = new MockService(serverless);
     const actualResourceGroupName = mockService.getResourceGroupName();
 
-    expect(actualResourceGroupName).toEqual(sls.service.provider["resourceGroup"]);
+    expect(actualResourceGroupName).toEqual(serverless.service.provider["resourceGroup"]);
   });
 
   it("Generates resource group from convention when NOT defined in sls yaml", () => {
-    sls.service.provider["resourceGroup"] = null;
-    const mockService = new MockService(sls);
+    serverless.service.provider["resourceGroup"] = null;
+    const mockService = new MockService(serverless);
     const actualResourceGroupName = mockService.getResourceGroupName();
     const expectedRegion = AzureNamingService.createShortAzureRegionName(mockService.getRegion());
     const expectedStage = AzureNamingService.createShortStageName(mockService.getStage());
-    const expectedResourceGroupName = `sls-${expectedRegion}-${expectedStage}-${sls.service["service"]}-rg`;
+    const expectedResourceGroupName = `sls-${expectedRegion}-${expectedStage}-${serverless.service["service"]}-rg`;
 
     expect(actualResourceGroupName).toEqual(expectedResourceGroupName);
   });
 
   it("set default prefix when one is not defined in yaml config", () => {
-    const mockService = new MockService(sls);
+    const mockService = new MockService(serverless);
     const actualPrefix = mockService.getPrefix();
     expect(actualPrefix).toEqual("sls");
   });
 
   it("use the prefix defined in sls yaml config", () => {
     const expectedPrefix = "testPrefix"
-    sls.service.provider["prefix"] = expectedPrefix;
-    const mockService = new MockService(sls);
+    serverless.service.provider["prefix"] = expectedPrefix;
+    const mockService = new MockService(serverless);
     const actualPrefix = mockService.getPrefix();
 
     expect(actualPrefix).toEqual(expectedPrefix);
   });
 
   it("Fails if credentials have not been set in serverless config", () => {
-    sls.variables["azureCredentials"] = null;
-    expect(() => new MockService(sls)).toThrow()
+    serverless.variables["azureCredentials"] = null;
+    expect(() => new MockService(serverless)).toThrow()
   });
 
   it("Makes HTTP request via axios", async () => {
@@ -202,36 +205,74 @@ describe("Base Service", () => {
 
   it("sets stage name from CLI", async () => {
     const stage = "test";
-    delete (sls.service as any as ServerlessAzureConfig).provider.resourceGroup;
-    expect(sls.service.provider.stage).not.toEqual(stage);
-    service = new MockService(sls, { stage } as any);
+    delete (serverless.service as any as ServerlessAzureConfig).provider.resourceGroup;
+    expect(serverless.service.provider.stage).not.toEqual(stage);
+    service = new MockService(serverless, { stage } as any);
     expect(service.getStage()).toEqual(stage);
     expect(service.getResourceGroupName()).toEqual(`sls-wus-${stage}-${serviceName}-rg`);
   });
 
   it("sets region name from CLI", async () => {
     const region = "East US";
-    delete (sls.service as any as ServerlessAzureConfig).provider.resourceGroup;
-    expect(sls.service.provider.region).not.toEqual(region);
-    service = new MockService(sls, { region } as any);
+    delete (serverless.service as any as ServerlessAzureConfig).provider.resourceGroup;
+    expect(serverless.service.provider.region).not.toEqual(region);
+    service = new MockService(serverless, { region } as any);
     expect(service.getRegion()).toEqual(region);
     expect(service.getResourceGroupName()).toEqual(`sls-eus-dev-${serviceName}-rg`);
   });
 
   it("sets prefix from CLI", async () => {
     const prefix = "prefix";
-    delete (sls.service as any as ServerlessAzureConfig).provider.resourceGroup;
-    expect(sls.service.provider["prefix"]).not.toEqual(prefix);
-    service = new MockService(sls, { prefix } as any);
+    delete (serverless.service as any as ServerlessAzureConfig).provider.resourceGroup;
+    expect(serverless.service.provider["prefix"]).not.toEqual(prefix);
+    service = new MockService(serverless, { prefix } as any);
     expect(service.getPrefix()).toEqual(prefix);
     expect(service.getResourceGroupName()).toEqual(`${prefix}-wus-dev-${serviceName}-rg`);
   });
 
   it("sets resource group from CLI", async () => {
     const resourceGroup = "resourceGroup";
-    delete (sls.service as any as ServerlessAzureConfig).provider.resourceGroup;
-    expect(sls.service.provider["resourceGroup"]).not.toEqual(resourceGroup);
-    service = new MockService(sls, { resourceGroup } as any);
+    delete (serverless.service as any as ServerlessAzureConfig).provider.resourceGroup;
+    expect(serverless.service.provider["resourceGroup"]).not.toEqual(resourceGroup);
+    service = new MockService(serverless, { resourceGroup } as any);
     expect(service.getResourceGroupName()).toEqual(resourceGroup);
   });
+
+  const cliSubscriptionId = "cli sub id";
+  const configSubscriptionId = "config sub id";
+
+  it("sets subscription ID from CLI", async () => {
+    process.env.azureSubId = envVarSubscriptionId;
+    serverless.service.provider["subscriptionId"] = configSubscriptionId;
+    serverless.variables["subscriptionId"] = loginResultSubscriptionId
+    service = new MockService(serverless, { subscriptionId: cliSubscriptionId } as any);
+    expect(service.getSubscriptionId()).toEqual(cliSubscriptionId);
+    expect(serverless.service.provider["subscriptionId"]).toEqual(cliSubscriptionId);
+  });
+
+  it("sets subscription ID from environment variable", async () => {
+    process.env.azureSubId = envVarSubscriptionId;
+    serverless.service.provider["subscriptionId"] = configSubscriptionId;
+    serverless.variables["subscriptionId"] = loginResultSubscriptionId
+    service = new MockService(serverless, { } as any);
+    expect(service.getSubscriptionId()).toEqual(envVarSubscriptionId);
+    expect(serverless.service.provider["subscriptionId"]).toEqual(envVarSubscriptionId);
+  });
+
+  it("sets subscription ID from config", async () => {
+    delete process.env.azureSubId;
+    serverless.service.provider["subscriptionId"] = configSubscriptionId;
+    serverless.variables["subscriptionId"] = loginResultSubscriptionId
+    service = new MockService(serverless, { } as any);
+    expect(service.getSubscriptionId()).toEqual(configSubscriptionId);
+    expect(serverless.service.provider["subscriptionId"]).toEqual(configSubscriptionId);
+  });
+
+  it("sets subscription ID from login result", async () => {
+    delete process.env.azureSubId;
+    serverless.variables["subscriptionId"] = loginResultSubscriptionId
+    service = new MockService(serverless, { } as any);
+    expect(service.getSubscriptionId()).toEqual(loginResultSubscriptionId);
+    expect(serverless.service.provider["subscriptionId"]).toEqual(loginResultSubscriptionId);
+  });
 });
diff --git a/src/services/functionAppService.test.ts b/src/services/functionAppService.test.ts
index b7cc9c8..aa70c80 100644
--- a/src/services/functionAppService.test.ts
+++ b/src/services/functionAppService.test.ts
@@ -16,6 +16,7 @@ jest.mock("@azure/arm-resources");
 jest.mock("./azureBlobStorageService");
 import { AzureBlobStorageService } from "./azureBlobStorageService"
 import configConstants from "../config";
+import { Utils } from "../shared/utils";
 
 describe("Function App Service", () => {
   const app = MockFactory.createTestSite();
@@ -41,8 +42,10 @@ describe("Function App Service", () => {
     setting2: "value2",
   }
 
-  beforeAll(() => {
-    const axiosMock = new MockAdapter(axios);
+  let axiosMock: MockAdapter;
+
+  beforeEach(() => {
+    axiosMock = new MockAdapter(axios);
 
     // Master Key
     axiosMock.onGet(masterKeyUrl).reply(200, { value: masterKey });
@@ -64,9 +67,7 @@ describe("Function App Service", () => {
         "serviceName.zip": "contents",
       }
     }, { createCwd: true, createTmp: true });
-  });
 
-  beforeEach(() => {
     WebSiteManagementClient.prototype.webApps = {
       get: jest.fn(() => app),
       deleteFunction: jest.fn(),
@@ -152,6 +153,30 @@ describe("Function App Service", () => {
     expect(await service.listFunctions(app)).toEqual(functionsResponse.map((f) => f.properties));
   });
 
+  it("list functions with retry", async () => {
+    axiosMock.onGet(listFunctionsUrl).reply(200, { value: [] });
+    const service = createService();
+
+    const originalRunWithRetry = Utils.runWithRetry;
+    Utils.runWithRetry = jest.fn(() => {
+      return {
+        data: {
+          value: []
+        }
+      }
+    }) as any;
+
+    await service.listFunctions(app);
+    const runWithRetryCalls = (Utils.runWithRetry as any).mock.calls;
+    Utils.runWithRetry = originalRunWithRetry;
+
+    expect(runWithRetryCalls).toHaveLength(1);
+    const call = runWithRetryCalls[0];
+    await expect(call[0]()).rejects.toThrow();
+    expect(call[1]).toEqual(30);
+    expect(call[2]).toEqual(30000);
+  });
+
   describe("Deployments", () => {
     const expectedDeployment: ArmDeployment = {
       parameters: {},
diff --git a/src/services/loginService.test.ts b/src/services/loginService.test.ts
index cfbc7bd..032d904 100644
--- a/src/services/loginService.test.ts
+++ b/src/services/loginService.test.ts
@@ -7,9 +7,19 @@ import * as nodeauth from "@azure/ms-rest-nodeauth";
 
 jest.mock("../plugins/login/utils/simpleFileTokenCache");
 import { SimpleFileTokenCache } from "../plugins/login/utils/simpleFileTokenCache";
+import { MockFactory } from "../test/mockFactory";
 
 describe("Login Service", () => {
 
+  let loginService: AzureLoginService;
+
+  beforeEach(() => {
+    loginService = new AzureLoginService(
+      MockFactory.createTestServerless(),
+      MockFactory.createTestServerlessOptions()
+    )
+  })
+
   it("logs in interactively with no cached login", async () => {
     // Ensure env variables are not set
     delete process.env.azureSubId;
@@ -25,7 +35,7 @@ describe("Login Service", () => {
       { value: jest.fn(() => emptyObj) }
     );
 
-    await AzureLoginService.login();
+    await loginService.login();
     expect(SimpleFileTokenCache).toBeCalled();
     expect(open).toBeCalledWith("https://microsoft.com/devicelogin");
     expect(nodeauth.interactiveLoginWithAuthResponse).toBeCalled();
@@ -42,7 +52,7 @@ describe("Login Service", () => {
     SimpleFileTokenCache.prototype.isEmpty = jest.fn(() => false);
     SimpleFileTokenCache.prototype.first = jest.fn(() => ({ userId: "" }));
 
-    await AzureLoginService.login();
+    await loginService.login();
     expect(SimpleFileTokenCache).toBeCalled();
     expect(nodeauth.DeviceTokenCredentials).toBeCalled();
     expect(SimpleFileTokenCache.prototype.listSubscriptions).toBeCalled();
@@ -55,7 +65,7 @@ describe("Login Service", () => {
     process.env.azureServicePrincipalPassword = "azureServicePrincipalPassword";
     process.env.azureServicePrincipalTenantId = "azureServicePrincipalTenantId";
 
-    await AzureLoginService.login();
+    await loginService.login();
     expect(nodeauth.loginWithServicePrincipalSecretWithAuthResponse).toBeCalledWith(
       "azureServicePrincipalClientId",
       "azureServicePrincipalPassword",
diff --git a/src/services/rollbackService.test.ts b/src/services/rollbackService.test.ts
index e5e0ce1..f532b22 100644
--- a/src/services/rollbackService.test.ts
+++ b/src/services/rollbackService.test.ts
@@ -5,6 +5,7 @@ import { ArmDeployment } from "../models/armTemplates";
 import { DeploymentConfig } from "../models/serverless";
 import { MockFactory } from "../test/mockFactory";
 import { RollbackService } from "./rollbackService";
+import fs from "fs";
 
 jest.mock("./azureBlobStorageService");
 import { AzureBlobStorageService } from "./azureBlobStorageService";
@@ -31,9 +32,10 @@ describe("Rollback Service", () => {
   const containerName = "deployment-artifacts";
   const artifactName = MockFactory.createTestDeployment().name.replace(
     configConstants.naming.suffix.deployment, configConstants.naming.suffix.artifact) + ".zip";
-  const artifactPath = `.serverless${path.sep}${artifactName}`
+  const artifactPath = path.join(".serverless", artifactName);
   const armDeployment: ArmDeployment = { template, parameters };
   const deploymentString = "deployments";
+  let unlinkSpy: jest.SpyInstance;
 
   function createOptions(timestamp?: string): Serverless.Options {
     return {
@@ -64,10 +66,12 @@ describe("Rollback Service", () => {
     ResourceService.prototype.listDeployments = jest.fn(() => Promise.resolve(deploymentString))
     AzureBlobStorageService.prototype.generateBlobSasTokenUrl = jest.fn(() => sasURL) as any;
     FunctionAppService.prototype.get = jest.fn(() => appStub) as any;
+    unlinkSpy = jest.spyOn(fs, "unlinkSync");
   });
 
   afterEach(() => {
     mockFs.restore();
+    unlinkSpy.mockRestore();
     jest.resetAllMocks();
   });
 
@@ -76,11 +80,8 @@ describe("Rollback Service", () => {
     const options = {} as any;
     const service = createService(sls, options);
     await service.rollback();
-    expect(sls.cli.log).lastCalledWith(
-      deploymentString,
-      undefined,
-      undefined
-    );
+    const logCalls: any[][] = (sls.cli.log as any).mock.calls;
+    expect(logCalls[logCalls.length - 1][0].endsWith(deploymentString)).toBe(true);
   });
 
   it("should return early with invalid timestamp", async () => {
@@ -94,6 +95,11 @@ describe("Rollback Service", () => {
   });
 
   it("should deploy blob package directly to function app", async () => {
+    const fsConfig = {};
+    fsConfig[artifactPath] = "contents";
+    // Mocking the existence of the downloaded artifact because the downloadBinary
+    // method won't write to the mock file system
+    mockFs(fsConfig);
     const service = createService();
     await service.rollback();
     expect(AzureBlobStorageService.prototype.initialize).toBeCalled();
@@ -107,7 +113,8 @@ describe("Rollback Service", () => {
     expect(FunctionAppService.prototype.uploadZippedArfifactToFunctionApp).toBeCalledWith(
       appStub,
       artifactPath
-    )
+    );
+    expect(unlinkSpy).toBeCalledWith(artifactPath);
   });
 
   it("should deploy function app with SAS URL", async () => {
@@ -133,5 +140,6 @@ describe("Rollback Service", () => {
     );
     expect(FunctionAppService.prototype.get).not.toBeCalled();
     expect(FunctionAppService.prototype.uploadZippedArfifactToFunctionApp).not.toBeCalled();
+    expect(unlinkSpy).not.toBeCalled();
   });
 });
diff --git a/src/test/mockFactory.ts b/src/test/mockFactory.ts
index d6c1a68..b91e4f8 100644
--- a/src/test/mockFactory.ts
+++ b/src/test/mockFactory.ts
@@ -16,6 +16,7 @@ import { ArmDeployment, ArmResourceTemplate, ArmTemplateProvisioningState } from
 import { ServicePrincipalEnvVariables } from "../models/azureProvider";
 import { Logger } from "../models/generic";
 import { ServerlessAzureConfig, ServerlessAzureProvider, ServerlessAzureFunctionConfig, ServerlessCliCommand } from "../models/serverless";
+import configConstants from "../config";
 
 function getAttribute(object: any, prop: string, defaultValue: any): any {
   if (object && object[prop]) {
@@ -174,7 +175,7 @@ export class MockFactory {
     const result = [];
     const originalTimestamp = +MockFactory.createTestTimestamp();
     for (let i = 0; i < count; i++) {
-      const name = (includeTimestamp) ? `deployment${i + 1}-t${originalTimestamp + i}` : `deployment${i + 1}`;
+      const name = (includeTimestamp) ? `${configConstants.naming.suffix.deployment}${i + 1}-t${originalTimestamp + i}` : `deployment${i + 1}`;
       result.push(
         MockFactory.createTestDeployment(name, i)
       )
@@ -194,7 +195,7 @@ export class MockFactory {
 
   public static createTestDeployment(name?: string, second: number = 0): DeploymentExtended {
     return {
-      name: name || `deployment1-t${MockFactory.createTestTimestamp()}`,
+      name: name || `${configConstants.naming.suffix.deployment}1-t${MockFactory.createTestTimestamp()}`,
       properties: {
         timestamp: new Date(2019, 1, 1, 0, 0, second),
         parameters: MockFactory.createTestParameters(),