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

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from 18 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
24 changes: 19 additions & 5 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 processEnv?: NodeJS.ProcessEnv;

public constructor(
capability: Capability,
Expand All @@ -74,8 +76,10 @@ export abstract class CaseFactory {
skipDeploy?: boolean;
skipValidate?: boolean;
skipPackage?: boolean;
skipErrorMessage?: string;
} = {},
custimized?: Record<string, string>
custimized?: Record<string, string>,
processEnv?: NodeJS.ProcessEnv
) {
this.capability = capability;
this.testPlanCaseId = testPlanCaseId;
Expand All @@ -84,6 +88,7 @@ export abstract class CaseFactory {
this.programmingLanguage = programmingLanguage;
this.options = options;
this.custimized = custimized;
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>
custimized?: Record<string, string>,
processEnv?: NodeJS.ProcessEnv
): Promise<void> {
await Executor.createProject(
testFolder,
appName,
capability,
programmingLanguage ? programmingLanguage : ProgrammingLanguage.TS,
custimized
custimized,
processEnv
);
}

Expand All @@ -127,6 +134,7 @@ export abstract class CaseFactory {
programmingLanguage,
options,
custimized,
processEnv,
onBefore,
onAfter,
onAfterCreate,
Expand All @@ -153,7 +161,8 @@ export abstract class CaseFactory {
testFolder,
capability,
programmingLanguage,
custimized
custimized,
Copy link
Contributor

Choose a reason for hiding this comment

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

typo -> customized

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks!

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,32 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

/**
* @author Yimin Jin <[email protected]>
*/

import { ProgrammingLanguage } from "@microsoft/teamsfx-core";
import { replaceSecretKey, validateFiles } from "./helper";
import { CopilotPluginCommonTest } from "./copilotPluginCommonTest";
import * as path from "path";

class CopilotPluginWithNoneAuthForJsCase extends CopilotPluginCommonTest {
public override async onAfterCreate(projectPath: string): Promise<void> {
const files: string[] = [
"appPackage/ai-plugin.json",
"appPackage/manifest.json",
"GenerateApiKey.ps1",
];
await validateFiles(projectPath, files);

const userFile = path.resolve(projectPath, "env", `.env.dev.user`);
await replaceSecretKey(userFile);
}
}

new CopilotPluginWithNoneAuthForJsCase(
28641218,
"[email protected]",
"api-key",
ProgrammingLanguage.CSharp
).test();
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

/**
* @author Yimin Jin <[email protected]>
*/

import { ProgrammingLanguage } from "@microsoft/teamsfx-core";
import { replaceSecretKey, validateFiles } from "./helper";
import { CopilotPluginCommonTest } from "./copilotPluginCommonTest";
import * as path from "path";

class CopilotPluginWithNoneAuthForJsCase extends CopilotPluginCommonTest {
public override async onAfterCreate(projectPath: string): Promise<void> {
const files: string[] = [
"appPackage/ai-plugin.json",
"appPackage/manifest.json",
"src/keyGen.js",
];
await validateFiles(projectPath, files);

const userFile = path.resolve(projectPath, "env", `.env.dev.user`);
await replaceSecretKey(userFile);
}
}

new CopilotPluginWithNoneAuthForJsCase(
Yimin-Jin marked this conversation as resolved.
Show resolved Hide resolved
28640069,
"[email protected]",
"api-key",
ProgrammingLanguage.JS
).test();
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

/**
* @author Yimin Jin <[email protected]>
*/

import { ProgrammingLanguage } from "@microsoft/teamsfx-core";
import { replaceSecretKey, validateFiles } from "./helper";
import { CopilotPluginCommonTest } from "./copilotPluginCommonTest";
import * as path from "path";

class CopilotPluginWithNoneAuthForTsCase extends CopilotPluginCommonTest {
public override async onAfterCreate(projectPath: string): Promise<void> {
const files: string[] = [
"appPackage/ai-plugin.json",
"appPackage/manifest.json",
"src/keyGen.ts",
];
await validateFiles(projectPath, files);

const userFile = path.resolve(projectPath, "env", `.env.dev.user`);
await replaceSecretKey(userFile);
}
}

new CopilotPluginWithNoneAuthForTsCase(
28640069,
"[email protected]",
"api-key",
ProgrammingLanguage.TS
).test();
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

/**
* @author Yimin Jin <[email protected]>
*/

import { ProgrammingLanguage } from "@microsoft/teamsfx-core";
import { validateFiles } from "./helper";
import { CopilotPluginCommonTest } from "./copilotPluginCommonTest";
class CopilotPluginWithNoneAuthForCsharpCase extends CopilotPluginCommonTest {
public override async onAfterCreate(projectPath: string): Promise<void> {
const files: string[] = [
"appPackage/ai-plugin.json",
"appPackage/manifest.json",
];
await validateFiles(projectPath, files);
}
}

new CopilotPluginWithNoneAuthForCsharpCase(
28641262,
"[email protected]",
"none",
ProgrammingLanguage.CSharp
).test();
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

/**
* @author Yimin Jin <[email protected]>
*/

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

class CopilotPluginWithNoneAuthForJsCase extends CopilotPluginCommonTest {
Yimin-Jin marked this conversation as resolved.
Show resolved Hide resolved
public override async onAfterCreate(projectPath: string): Promise<void> {
const files: string[] = [
"appPackage/ai-plugin.json",
"appPackage/manifest.json",
];
await validateFiles(projectPath, files);
}
}

new CopilotPluginWithNoneAuthForJsCase(
27569734,
"[email protected]",
"none",
ProgrammingLanguage.JS
).test();
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

/**
* @author Yimin Jin <[email protected]>
*/

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

class CopilotPluginWithNoneAuthForTsCase extends CopilotPluginCommonTest {
public override async onAfterCreate(projectPath: string): Promise<void> {
const files: string[] = [
"appPackage/ai-plugin.json",
"appPackage/manifest.json",
];
await validateFiles(projectPath, files);
}
}

new CopilotPluginWithNoneAuthForTsCase(
27569734,
"[email protected]",
"none",
ProgrammingLanguage.TS
).test();
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

/**
* @author Hui Miao <[email protected]>
*/

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

class CopilotPluginOAuthForCsharpTestCase extends CopilotPluginCommonTest {
public override async onAfterCreate(projectPath: string): Promise<void> {
const files: string[] = [
"appPackage/ai-plugin.dev.json",
"appPackage/manifest.json",
];
await validateFiles(projectPath, files);
}
}

new CopilotPluginOAuthForCsharpTestCase(
28641204,
"[email protected]",
"oauth",
ProgrammingLanguage.CSharp
).test();
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

/**
* @author Hui Miao <[email protected]>
*/

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

class CopilotPluginOAuthForJsTestCase extends CopilotPluginCommonTest {
public override async onAfterCreate(projectPath: string): Promise<void> {
const files: string[] = [
"appPackage/ai-plugin.dev.json",
"appPackage/manifest.json",
];
await validateFiles(projectPath, files);
}
}

new CopilotPluginOAuthForJsTestCase(
27569691,
"[email protected]",
"oauth",
ProgrammingLanguage.JS
).test();
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

/**
* @author Hui Miao <[email protected]>
*/

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

class CopilotPluginOAuthForTsTestCase extends CopilotPluginCommonTest {
public override async onAfterCreate(projectPath: string): Promise<void> {
const files: string[] = [
"appPackage/ai-plugin.dev.json",
"appPackage/manifest.json",
];
await validateFiles(projectPath, files);
}
}

new CopilotPluginOAuthForTsTestCase(
27569691,
"[email protected]",
"oauth",
ProgrammingLanguage.TS
).test();
Loading
Loading