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.

Sign up for GitHub

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

[Azure Monitor OpenTelemetry] Use dev-tool to run tests #28805

Merged
merged 6 commits into from
Mar 7, 2024
Merged

[Azure Monitor OpenTelemetry] Use dev-tool to run tests #28805

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
052b5e0
Use dev-tool to run the tests
hectorhdzg Mar 6, 2024
71db4bb
format
hectorhdzg Mar 6, 2024
f2dc18f
rush update
hectorhdzg Mar 6, 2024
5aa5e6e
rush update
hectorhdzg Mar 6, 2024
ea0a5a6
Adding build:test
hectorhdzg Mar 6, 2024
db6070e
Add timeout to test execution
hectorhdzg Mar 6, 2024
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
9 changes: 5 additions & 4 deletions common/config/rush/pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 7 additions & 8 deletions sdk/monitor/monitor-opentelemetry-exporter/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
"scripts": {
"clean": "rimraf dist-esm types dist",
"build:browser": "echo skipped",
"build:test": "echo skipped",
"build:node": "tsc -p . && dev-tool run bundle --browser-test=false",
"build:samples": "dev-tool samples publish --force",
"build:test": "tsc -p . && dev-tool run bundle --browser-test=false",
"build:samples": "echo Obsolete.",
"build": "npm run build:node && npm run build:browser && api-extractor run --local",
"check-format": "dev-tool run vendored prettier --list-different --config ../../../.prettierrc.json --ignore-path ../../../.prettierignore \"src/**/*.ts\" \"test/**/*.ts\" \"samples-dev/**/*.ts\" \"*.{js,json}\"",
"execute:samples": "dev-tool samples run samples-dev",
Expand All @@ -21,17 +21,16 @@
"generate:client": "autorest --typescript ./swagger/README.md",
"lint:fix": "eslint package.json api-extractor.json src test --ext .ts --fix --fix-type [problem,suggestion]",
"lint": "eslint package.json api-extractor.json src test --ext .ts -f html -o telemetry-exporter-lintReport.html || exit 0",
"test": "npm run test:node && npm run test:browser",
"test:node": "npm run unit-test:node",
"test": "npm run clean && npm run build:test && npm run unit-test",
"test:node": "npm run clean && npm run build:test && npm run unit-test:node",
"test:browser": "npm run unit-test:browser",
"unit-test:browser": "echo skipped",
"unit-test:node": "mocha -r ../../../common/tools/esm-workaround.js -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --reporter-option output=test-results.xml --timeout 1200000 --full-trace \"dist-esm/test/internal/**/*.test.js\"",
"unit-test:node:debug": "mocha --inspect-brk -r ../../../common/tools/esm-workaround.js -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --reporter-option output=test-results.xml --timeout 1200000 --full-trace \"dist-esm/test/internal/**/*.test.js\"",
"unit-test:node": "dev-tool run test:node-ts-input --no-test-proxy=true -- --timeout 1200000 \"test/internal/**/*.test.ts\"",
Copy link
Member

@HarshaNalluru HarshaNalluru Mar 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is good, but just a little heads up..
we'll be moving to vitest soon for all the packages instead of mocha.

And while the tests are not migrated to vitest yet, we'll have to follow this #28801 in order to consume the core that is upgraded to esm, that would also fix the failures you observed.

PS: I don't have any concern on which commands we use in the short-term as long as the pipelines are green.

"unit-test:node:debug": "dev-tool run test:node-ts-input --no-test-proxy=true -- --inspect-brk \"test/internal/**/*.test.ts\"",
"unit-test:node:no-timeout": "echo skipped",
"unit-test": "npm run unit-test:node && npm run unit-test:browser",
"functional-test": "mocha -r ../../../common/tools/esm-workaround.js -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --reporter-option output=test-results.xml --timeout 1200000 --full-trace \"dist-esm/test/internal/functional/*.test.js\"",
"integration-test:browser": "echo skipped",
"integration-test:node": "npm run functional-test",
"integration-test:node": "dev-tool run test:node-ts-input --no-test-proxy=true -- --timeout 1200000 \"test/internal/functional/**/*.test.ts\"",
"integration-test": "npm run integration-test:node && npm run integration-test:browser",
"report": "nyc report --reporter=json",
"test-opentelemetry-versions": "node test-opentelemetry-versions.js 2>&1",
Expand Down
2 changes: 1 addition & 1 deletion sdk/monitor/monitor-opentelemetry-exporter/test/internal/metricUtil.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { Tags } from "../../src/types";
import { Context, getInstance } from "../../src/platform";

const context = getInstance();
const packageJsonPath = path.resolve(__dirname, "../../../", "./package.json");
const packageJsonPath = path.resolve(__dirname, "../../", "./package.json");
const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, "utf8"));

let testMetrics: ResourceMetrics;
Expand Down
2 changes: 1 addition & 1 deletion sdk/monitor/monitor-opentelemetry-exporter/test/internal/spanUtils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const tracerProviderConfig: TracerConfig = {
};

const tracer = new BasicTracerProvider(tracerProviderConfig).getTracer("default");
const packageJsonPath = path.resolve(__dirname, "../../../", "./package.json");
const packageJsonPath = path.resolve(__dirname, "../../", "./package.json");
const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, "utf8"));

function assertEnvelope(
Expand Down
22 changes: 11 additions & 11 deletions sdk/monitor/monitor-opentelemetry/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,28 @@
"module": "dist-esm/src/index.js",
"types": "types/monitor-opentelemetry.d.ts",
"scripts": {
"clean": "rimraf dist-esm types dist",
"build:samples": "echo Obsolete.",
"build:browser": "echo skipped",
"build:test": "echo skipped",
"build:node": "tsc -p . && dev-tool run bundle --browser-test=false",
"build:samples": "dev-tool samples publish --force",
"build:test": "tsc -p . && dev-tool run bundle --browser-test=false",
"build": "npm run build:node && npm run build:browser && api-extractor run --local",
"check-format": "dev-tool run vendored prettier --list-different --config ../../../.prettierrc.json --ignore-path ../../../.prettierignore \"src/**/*.ts\" \"test/**/*.ts\" \"samples-dev/**/*.ts\" \"*.{js,json}\"",
"clean": "rimraf dist-esm types dist",
"execute:samples": "dev-tool samples run samples-dev",
"extract-api": "tsc -p . && api-extractor run --local",
"format": "dev-tool run vendored prettier --write --config ../../../.prettierrc.json --ignore-path ../../../.prettierignore \"src/**/*.ts\" \"test/**/*.ts\" \"samples-dev/**/*.ts\" \"*.{js,json}\"",
"generate:client": "autorest --typescript ./swagger/README.md",
"lint:fix": "eslint package.json api-extractor.json src test --ext .ts --fix --fix-type [problem,suggestion]",
"lint": "eslint package.json api-extractor.json src test --ext .ts -f html -o telemetry-lintReport.html || exit 0",
"test": "npm run test:node && npm run test:browser",
"test:node": "npm run unit-test:node",
"test:node": "npm run clean && npm run build:test && npm run unit-test:node",
"test": "npm run clean && npm run build:test && npm run unit-test",
"test:browser": "npm run unit-test:browser",
"unit-test:browser": "echo skipped",
"unit-test:node": "mocha -r ../../../common/tools/esm-workaround -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --reporter-option output=test-results.xml --timeout 1200000 --full-trace \"dist-esm/test/internal/unit/**/*.test.js\"",
"unit-test:node:debug": "mocha --inspect-brk -r ../../../common/tools/esm-workaround -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --reporter-option output=test-results.xml --timeout 1200000 --full-trace \"dist-esm/test/internal/unit/**/*.test.js\"",
"unit-test:node:no-timeout": "echo skipped",
"unit-test:node": "dev-tool run test:node-ts-input --no-test-proxy=true -- \"test/internal/unit/**/*.test.ts\"",
"unit-test:node:debug": "dev-tool run test:node-ts-input --no-test-proxy=true -- --inspect-brk \"test/internal/unit/**/*.test.ts\"",
"unit-test": "npm run unit-test:node && npm run unit-test:browser",
"functional-test": "mocha -r ../../../common/tools/esm-workaround -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --reporter-option output=test-results.xml --timeout 1200000 --full-trace \"dist-esm/test/internal/functional/*.test.js\"",
"integration-test:browser": "echo skipped",
"integration-test:node": "npm run functional-test",
"integration-test:node": "dev-tool run test:node-ts-input --no-test-proxy=true -- \"test/internal/functional/**/*.test.ts\"",
"integration-test": "npm run integration-test:node && npm run integration-test:browser",
"report": "nyc report --reporter=json",
"test-opentelemetry-versions": "node test-opentelemetry-versions.js 2>&1",
Expand Down Expand Up @@ -68,6 +66,7 @@
"devDependencies": {
"@azure/dev-tool": "^1.0.0",
"@azure/eslint-plugin-azure-sdk": "^3.0.0",
"@azure/test-utils": "^1.0.0",
"@microsoft/api-extractor": "^7.31.1",
"@types/mocha": "^10.0.0",
"@types/node": "^18.0.0",
Expand All @@ -78,6 +77,7 @@
"mocha": "^10.0.0",
"nock": "^12.0.3",
"c8": "^8.0.0",
"cross-env": "^7.0.3",
"rimraf": "^3.0.0",
"sinon": "^17.0.0",
"ts-node": "^10.0.0",
Expand Down Expand Up @@ -111,7 +111,7 @@
"@opentelemetry/sdk-trace-node": "^1.21.0",
"@opentelemetry/semantic-conventions": "^1.21.0",
"tslib": "^2.2.0",
"@microsoft/applicationinsights-web-snippet": "^1.0.1",
"@microsoft/applicationinsights-web-snippet": "1.0.1",
"@opentelemetry/resource-detector-azure": "^0.2.4"
},
"sideEffects": false,
Expand Down
15 changes: 7 additions & 8 deletions sdk/monitor/monitor-opentelemetry/src/shared/jsonConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export class JsonConfig implements AzureMonitorOpenTelemetryOptions {

private static _instance: JsonConfig;

private _tempDir: string;

/** Get Singleton instance */
public static getInstance() {
if (!JsonConfig._instance) {
Expand All @@ -48,11 +50,8 @@ export class JsonConfig implements AzureMonitorOpenTelemetryOptions {
* Initializes a new instance of the JsonConfig class.
*/
constructor() {
this._loadJsonFile();
}

private _loadJsonFile() {
let jsonString = "";
this._tempDir = "";
const contentJsonConfig = process.env[ENV_CONTENT];
// JSON string added directly in env variable
if (contentJsonConfig) {
Expand All @@ -62,17 +61,17 @@ export class JsonConfig implements AzureMonitorOpenTelemetryOptions {
else {
let configFileName = "applicationinsights.json";
let rootPath = path.join(__dirname, "../../../"); // Root of folder (__dirname = ../dist-esm/src)
let tempDir = path.join(rootPath, configFileName); // default
this._tempDir = path.join(rootPath, configFileName); // default
let configFile = process.env[ENV_CONFIGURATION_FILE];
if (configFile) {
if (path.isAbsolute(configFile)) {
tempDir = configFile;
this._tempDir = configFile;
} else {
tempDir = path.join(rootPath, configFile); // Relative path to applicationinsights folder
this._tempDir = path.join(rootPath, configFile); // Relative path to applicationinsights folder
}
}
try {
jsonString = fs.readFileSync(tempDir, "utf8");
jsonString = fs.readFileSync(this._tempDir, "utf8");
} catch (err) {
Logger.getInstance().info("Failed to read JSON config file: ", err);
}
Expand Down
5 changes: 1 addition & 4 deletions sdk/monitor/monitor-opentelemetry/test/internal/unit/shared/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,7 @@ describe("Library/Config", () => {
it("merge JSON config", () => {
(JsonConfig["_instance"] as any) = undefined;
const env = <{ [id: string]: string }>{};
const customConfigJSONPath = path.resolve(
__dirname,
"../../../../../test/internal/unit/shared/config.json",
);
const customConfigJSONPath = path.resolve(__dirname, "config.json");
env["APPLICATIONINSIGHTS_CONFIGURATION_FILE"] = customConfigJSONPath; // Load JSON config
process.env = env;
const config = new InternalConfig();
Expand Down
25 changes: 6 additions & 19 deletions sdk/monitor/monitor-opentelemetry/test/internal/unit/shared/jsonConfig.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

import * as assert from "assert";
import * as sinon from "sinon";
import * as fs from "fs";
import * as path from "path";
import { JsonConfig } from "../../../../src/shared/jsonConfig";

Expand All @@ -28,20 +27,14 @@ describe("Json Config", () => {

describe("config path", () => {
it("Default file path", () => {
const fileSpy = sandbox.spy(fs, "readFileSync");
const config = JsonConfig.getInstance();
config["_loadJsonFile"]();
assert.ok(fileSpy.called);
const defaultPath = path.resolve(process.cwd(), "applicationinsights.json");
assert.deepStrictEqual(fileSpy.args[0][0], defaultPath);
let defaultPath = path.join(process.cwd(), "../", "applicationinsights.json");
assert.deepStrictEqual(config["_tempDir"], defaultPath);
});

it("Absolute file path", () => {
const env = <{ [id: string]: string }>{};
const customConfigJSONPath = path.resolve(
__dirname,
"../../../../../test/internal/unit/shared/config.json",
);
const customConfigJSONPath = path.resolve(__dirname, "config.json");
env["APPLICATIONINSIGHTS_CONFIGURATION_FILE"] = customConfigJSONPath;
process.env = env;
const config = JsonConfig.getInstance();
Expand All @@ -53,7 +46,7 @@ describe("Json Config", () => {

it("Relative file path", () => {
const env = <{ [id: string]: string }>{};
const customConfigJSONPath = "./test/internal/unit/shared/config.json";
const customConfigJSONPath = "monitor-opentelemetry/test/internal/unit/shared/config.json";
env["APPLICATIONINSIGHTS_CONFIGURATION_FILE"] = customConfigJSONPath;
process.env = env;
const config = JsonConfig.getInstance();
Expand All @@ -67,10 +60,7 @@ describe("Json Config", () => {
describe("configuration values", () => {
it("Should take configurations from JSON config file", () => {
const env = <{ [id: string]: string }>{};
const customConfigJSONPath = path.resolve(
__dirname,
"../../../../../test/internal/unit/shared/config.json",
);
const customConfigJSONPath = path.resolve(__dirname, "config.json");
env["APPLICATIONINSIGHTS_CONFIGURATION_FILE"] = customConfigJSONPath;
process.env = env;
const config = JsonConfig.getInstance();
Expand Down Expand Up @@ -103,10 +93,7 @@ describe("Json Config", () => {

it("Should take configurations from JSON config file over environment variables if both are configured", () => {
const env = <{ [id: string]: string }>{};
const customConfigJSONPath = path.resolve(
__dirname,
"../../../../../test/internal/unit/shared/config.json",
);
const customConfigJSONPath = path.resolve(__dirname, "config.json");
env["APPLICATIONINSIGHTS_CONFIGURATION_FILE"] = customConfigJSONPath;
env["APPLICATIONINSIGHTS_CONNECTION_STRING"] = "TestConnectionString";
process.env = env;
Expand Down