Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add e2e test #11941

Open
wants to merge 20 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions packages/tests/src/commonlib/functionValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as chai from "chai";
import glob from "glob";
import path from "path";
import { EnvConstants, PluginId, StateConfigKey } from "./constants";
// eslint-disable-next-line import/no-cycle
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible not to suppress this check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As some historical reasons, many files in the 'tests/' folder currently have nested import issues. If there are any PR modify any of these files, the pre-commit check will fail. The pre-commit check seems to be added later on.

We need to take a look at all those affected files and discuss with related owners on how we can fix them. Let me follow up with them.

import {
getResourceGroupNameFromResourceId,
getSiteNameFromResourceId,
Expand Down Expand Up @@ -39,6 +40,7 @@ enum BaseConfig {
API_ENDPOINT = "API_ENDPOINT",
M365_APPLICATION_ID_URI = "M365_APPLICATION_ID_URI",
IDENTITY_ID = "IDENTITY_ID",
WEBSITE_CONTENTSHARE = "WEBSITE_CONTENTSHARE",
}

enum SQLConfig {
Expand Down Expand Up @@ -110,14 +112,11 @@ export class FunctionValidator {
token as string
);
chai.assert.exists(webappSettingsResponse);
const endpoint =
(this.ctx[EnvConstants.FUNCTION_ENDPOINT] as string) ??
(this.ctx[EnvConstants.FUNCTION_ENDPOINT_2] as string);
chai.assert.equal(
webappSettingsResponse[BaseConfig.API_ENDPOINT],
endpoint
);

const contentShare =
webappSettingsResponse[BaseConfig.WEBSITE_CONTENTSHARE];
const functionNameInAzure = contentShare.split("-")[0];
chai.assert.equal(functionNameInAzure, this.functionAppName);
console.log("Successfully validate Function Provision.");
}

Expand Down
32 changes: 23 additions & 9 deletions packages/tests/src/e2e/caseFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,10 @@ export abstract class CaseFactory {
skipDeploy?: boolean;
skipValidate?: boolean;
skipPackage?: boolean;
skipErrorMessage?: string;
};
public custimized?: Record<string, string>;
public customized?: Record<string, string>;
public processEnv?: NodeJS.ProcessEnv;

public constructor(
capability: Capability,
Expand All @@ -74,16 +76,19 @@ export abstract class CaseFactory {
skipDeploy?: boolean;
skipValidate?: boolean;
skipPackage?: boolean;
skipErrorMessage?: string;
} = {},
custimized?: Record<string, string>
customized?: Record<string, string>,
processEnv?: NodeJS.ProcessEnv
) {
this.capability = capability;
this.testPlanCaseId = testPlanCaseId;
this.author = author;
this.validate = validate;
this.programmingLanguage = programmingLanguage;
this.options = options;
this.custimized = custimized;
this.customized = customized;
this.processEnv = processEnv;
}

public onBefore(): Promise<void> {
Expand All @@ -103,14 +108,16 @@ export abstract class CaseFactory {
testFolder: string,
capability: Capability,
programmingLanguage?: ProgrammingLanguage,
custimized?: Record<string, string>
customized?: Record<string, string>,
processEnv?: NodeJS.ProcessEnv
): Promise<void> {
await Executor.createProject(
testFolder,
appName,
capability,
programmingLanguage ? programmingLanguage : ProgrammingLanguage.TS,
custimized
customized,
processEnv
);
}

Expand All @@ -126,14 +133,15 @@ export abstract class CaseFactory {
validate,
programmingLanguage,
options,
custimized,
customized,
processEnv,
onBefore,
onAfter,
onAfterCreate,
onBeforeProvision,
onCreate,
} = this;
describe(`template Test: ${capability}`, function () {
describe(`template Test: ${capability} - ${programmingLanguage}`, function () {
const testFolder = getTestFolder();
const appName = getUniqueAppName();
const projectPath = path.resolve(testFolder, appName);
Expand All @@ -153,7 +161,8 @@ export abstract class CaseFactory {
testFolder,
capability,
programmingLanguage,
custimized
customized,
processEnv
);
expect(fs.pathExistsSync(projectPath)).to.be.true;

Expand All @@ -173,7 +182,12 @@ export abstract class CaseFactory {
expect(result).to.be.true;
process.env["AZURE_RESOURCE_GROUP_NAME"] = appName + "-rg";

const { success } = await Executor.provision(projectPath);
const { success } = await Executor.provision(
projectPath,
"dev",
true,
options?.skipErrorMessage
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if skip this error message, the pipeline may skip some action

Copy link
Contributor Author

@huimiu huimiu Jul 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there are some features that are in private preview. The test account cannot run some templates successfully (lacking some permissions), so we are going to skip this error message for now

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if so, maybe set skipErrorMessage for specific templates instead of set all rules?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only some templates have passed the skipErrorMessage parameter, while others that don't need it haven't. This parameter is optional. The current change provides an option for test cases to choose whether to skip certain actions. Do you think this change is ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't add this parameter, the CaseFactory cannot support scenarios that need to skip some error messages, then I think the CaseFactory is not reusable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like everyone has some concerns about this, so let's hold off on adding any test cases related to the Copilot plugin until they've fully rolled it out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this error is skipped, is it possible to simulate the actual e2e process?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can simulate most, but not all. We'll remove the skip logic once the feature is GA.

Copy link
Contributor Author

@huimiu huimiu Jul 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if not support provision, the remote for e2e may not stable for the test case, can we use local debug for testing instead of remote? if skip the provision error, the azure resource is really provisioned ?

Sorry, I didn't explain clearly enough. There is only one action, "extendToM365," will fail during provisioning, the arm deploy is work. Also, both teamsapp.local.yml and teamsapp.yml have the action extendToM365.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, there is a common function removeTeamsAppExtendToM365 to work around the issue.
https://github.com/OfficeDev/teams-toolkit/blob/dev/packages/tests/src/e2e/frontend/TestCreateTab.tests.ts#L67

);
expect(success).to.be.true;

// Validate Provision
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

/**
* @author Yimin Jin <yiminjin@microsoft.com>
*/

import { ProgrammingLanguage } from "@microsoft/teamsfx-core";
import { CopilotPluginCommonTest } from "./copilotPluginCommonTest";

class CopilotPluginWithApiKeyAuthCase extends CopilotPluginCommonTest {}
const validateFiles = {
[ProgrammingLanguage.JS]: [
"appPackage/ai-plugin.json",
"appPackage/manifest.json",
"src/keyGen.js",
],
[ProgrammingLanguage.TS]: [
"appPackage/ai-plugin.json",
"appPackage/manifest.json",
"src/keyGen.ts",
],
[ProgrammingLanguage.CSharp]: [
"appPackage/ai-plugin.json",
"appPackage/manifest.json",
"GenerateApiKey.ps1",
],
};
new CopilotPluginWithApiKeyAuthCase(
28640069,
"yimin@microsoft.com",
"api-key",
ProgrammingLanguage.JS,
validateFiles[ProgrammingLanguage.JS]
).test();

new CopilotPluginWithApiKeyAuthCase(
28640069,
"yimin@microsoft.com",
"api-key",
ProgrammingLanguage.TS,
validateFiles[ProgrammingLanguage.TS]
).test();

new CopilotPluginWithApiKeyAuthCase(
28640069,
"yimin@microsoft.com",
"api-key",
ProgrammingLanguage.CSharp,
validateFiles[ProgrammingLanguage.CSharp]
).test();
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

/**
* @author Yimin Jin <yiminjin@microsoft.com>
*/

import { ProgrammingLanguage } from "@microsoft/teamsfx-core";
import { CopilotPluginCommonTest } from "./copilotPluginCommonTest";

class CopilotPluginWithNoneAuthCase extends CopilotPluginCommonTest {}
const validateFiles = ["appPackage/ai-plugin.json", "appPackage/manifest.json"];

new CopilotPluginWithNoneAuthCase(
27569734,
"yimin@microsoft.com",
"none",
ProgrammingLanguage.JS,
validateFiles
).test();

new CopilotPluginWithNoneAuthCase(
27569734,
"yimin@microsoft.com",
"none",
ProgrammingLanguage.TS,
validateFiles
).test();

new CopilotPluginWithNoneAuthCase(
27569734,
"yimin@microsoft.com",
"none",
ProgrammingLanguage.CSharp,
validateFiles
).test();
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

/**
* @author Hui Miao <huimiao@microsoft.com>
*/

import { ProgrammingLanguage } from "@microsoft/teamsfx-core";
import { CopilotPluginCommonTest } from "./copilotPluginCommonTest";

class CopilotPluginWithOAuthCase extends CopilotPluginCommonTest {}
const validateFiles = [
"appPackage/ai-plugin.dev.json",
"appPackage/manifest.json",
];

new CopilotPluginWithOAuthCase(
28641204,
"huimiao@microsoft.com",
"oauth",
ProgrammingLanguage.JS,
validateFiles
).test();

new CopilotPluginWithOAuthCase(
28641204,
"huimiao@microsoft.com",
"oauth",
ProgrammingLanguage.TS,
validateFiles
).test();

new CopilotPluginWithOAuthCase(
28641204,
"huimiao@microsoft.com",
"oauth",
ProgrammingLanguage.CSharp,
validateFiles
).test();
61 changes: 61 additions & 0 deletions packages/tests/src/e2e/copilotplugin/copilotPluginCommonTest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

/**
* @author Hui Miao <huimiao@microsoft.com>
*/

import { Capability } from "../../utils/constants";
import { CaseFactory } from "../caseFactory";
import { ProgrammingLanguage } from "@microsoft/teamsfx-core";
import { replaceSecretKey, validateFiles } from "./helper";
import * as path from "path";
export class CopilotPluginCommonTest extends CaseFactory {
validateFileList?: string[];
authOption?: string;

public constructor(
testPlanCaseId: number,
author: string,
authOption: "none" | "api-key" | "oauth",
programmingLanguage?: ProgrammingLanguage,
validateFileList?: string[]
) {
const env = Object.assign({}, process.env);
env["DEVELOP_COPILOT_PLUGIN"] = "true";
if (programmingLanguage === ProgrammingLanguage.CSharp) {
env["TEAMSFX_CLI_DOTNET"] = "true";
}

const skipOptions = {
skipValidate: true,
skipErrorMessage: "No elements found in the manifest",
};

const authOptions: Record<string, string> = {};
authOptions["api-auth"] = authOption;

super(
Capability.CopilotPluginFromScratch,
testPlanCaseId,
author,
["function"],
programmingLanguage,
skipOptions,
authOptions,
env
);
this.validateFileList = validateFileList;
this.authOption = authOption;
this.onAfterCreate = this.onAfterCreate.bind(this);
}

public override async onAfterCreate(projectPath: string): Promise<void> {
await validateFiles(projectPath, this.validateFileList || []);

if (this.authOption === "api-key") {
const userFile = path.resolve(projectPath, "env", `.env.dev.user`);
await replaceSecretKey(userFile);
}
}
}
24 changes: 24 additions & 0 deletions packages/tests/src/e2e/copilotplugin/helper.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
import * as path from "path";
import * as fs from "fs-extra";
import { expect } from "chai";

export async function validateFiles(
projectPath: string,
files: string[]
): Promise<void> {
for (const file of files) {
const filePath = path.join(projectPath, file);
expect(fs.existsSync(filePath), `${filePath} must exist.`).to.eq(true);
}
console.log("Files validation successful");
}

export async function replaceSecretKey(userFile: string): Promise<void> {
const newSecretKey = 'SECRET_API_KEY="test-secret-api-key"';
let fileContent = fs.readFileSync(userFile, "utf8");
fileContent = fileContent.replace(/(SECRET_API_KEY=).*/, newSecretKey);
fs.writeFileSync(userFile, fileContent, "utf8");
console.log(`Updated ${newSecretKey} in .env.dev.user file`);
}
Loading
Loading