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

refactor(build-cli): Move tests alongside source #22684

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
3 changes: 2 additions & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@
{
"name": "fluid-build",
"program": "${workspaceFolder}/build-tools/packages/build-tools/bin/fluid-build",
"cwd": "${workspaceFolder}/packages/common/container-definitions",
// "cwd": "${workspaceFolder}/packages/common/container-definitions",
"cwd": "${workspaceFolder}/build-tools",
tylerbutler marked this conversation as resolved.
Show resolved Hide resolved
"request": "launch",
"skipFiles": ["<node_internals>/**"],
"type": "node",
Expand Down
3 changes: 3 additions & 0 deletions build-tools/packages/build-cli/.eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ module.exports = {
require.resolve("@fluidframework/eslint-config-fluid/recommended"),
"prettier",
],
parserOptions: {
project: ["./tsconfig.json", "./src/test/tsconfig.json"],
},
rules: {
// This rule is often triggered when using custom Flags, so disabling.
"object-shorthand": "off",
Expand Down
2 changes: 1 addition & 1 deletion build-tools/packages/build-cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
"build:docs": "api-extractor run --local",
"build:manifest": "oclif manifest",
"build:readme": "oclif readme --version 0.0.0 --multi --no-aliases",
"build:test": "tsc --project ./test/tsconfig.json",
"build:test": "tsc --project ./src/test/tsconfig.json",
"check:biome": "biome check .",
"check:format": "npm run check:biome",
"ci:build:docs": "api-extractor run",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ import { strict as assert } from "node:assert";
import {
loadTypesSourceFile,
typeDataFromFile,
} from "../../../src/commands/generate/typetests.js";
import type { TypeData } from "../../../src/typeValidator/typeData.js";
} from "../../../commands/generate/typetests.js";
// eslint-disable-next-line import/no-internal-modules
import type { TypeData } from "../../../typeValidator/typeData.js";

describe("generate:typetests", () => {
const logger = {
/* eslint-disable @typescript-eslint/explicit-function-return-type */
log: () => assert.fail(),
info: () => assert.fail(),
warning: () => assert.fail(),
Expand All @@ -20,6 +22,7 @@ describe("generate:typetests", () => {
logHr: () => assert.fail(),
logIndent: () => assert.fail(),
};
/* eslint-enable @typescript-eslint/explicit-function-return-type */

function forCompare(data: Map<string, TypeData>, includeTypeOf?: true): unknown[] {
return [...data.entries()].map(([k, v]) => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@
import { GitRepo, type Package, getResolvedFluidRoot } from "@fluidframework/build-tools";
import { expect } from "chai";

import { type PackageNamePolicyConfig } from "../../src/config.js";
import { Context } from "../../src/library/index.js";
import { type PackageNamePolicyConfig } from "../../config.js";
import { Context } from "../../library/index.js";
import {
type Feed,
feeds,
packagePublishesToFeed,
} from "../../src/library/repoPolicyCheck/npmPackages.js";
// eslint-disable-next-line import/no-internal-modules
} from "../../library/repoPolicyCheck/npmPackages.js";

/**
* Calculates the packages that should be published to a feed and returns a map of Feed to the packages that should be
Expand Down Expand Up @@ -42,14 +43,16 @@ function FeedsForPackages(
return mapping;
}

// eslint-disable-next-line @typescript-eslint/no-misused-promises
describe("feeds", async () => {
const resolvedRoot = await getResolvedFluidRoot();
const gitRepo = new GitRepo(resolvedRoot);
const branch = await gitRepo.getCurrentBranchName();

const context = new Context(gitRepo, "microsoft/FluidFramework", branch);
const config = context.flubConfig.policy?.packageNames!;
const packages = FeedsForPackages(context.packages, config);
const config = context.flubConfig.policy?.packageNames;
// eslint-disable-next-line new-cap, @typescript-eslint/no-non-null-assertion
const packages = FeedsForPackages(context.packages, config!);

it("dev and build feed are mutually exclusive", () => {
const dev = packages.get("internal-dev")?.map((p) => p.name);
Expand All @@ -59,6 +62,7 @@ describe("feeds", async () => {
return dev?.includes(name);
});

// eslint-disable-next-line @typescript-eslint/no-unused-expressions
expect(hasDupes).to.be.false;
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@
* Licensed under the MIT License.
*/

import { FluidReleaseMachine } from "../../src/machines/index.js";
import { FluidReleaseMachine } from "../../machines/index.js";
import { initializeCommandTestFunction } from "../init.js";

const test = initializeCommandTestFunction(import.meta.url);
const knownUnhandledStates: string[] = [
const knownUnhandledStates: Set<string> = new Set([
// Known unhandled states can be added here temporarily during development.
];
]);

const machineStates = FluidReleaseMachine.states()
.filter((s) => !knownUnhandledStates.includes(s))
.filter((s) => !knownUnhandledStates.has(s))
.sort();

describe("release command handles all states", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { ReleaseVersion, VersionBumpType } from "@fluid-tools/version-tools";
import chai, { expect } from "chai";
import assertArrays from "chai-arrays";

import { ReleaseGroup, ReleasePackage } from "../../../src/releaseGroups.js";
import { ReleaseGroup, ReleasePackage } from "../../../releaseGroups.js";
import { initializeCommandTestFunction } from "../../init.js";

const test = initializeCommandTestFunction(import.meta.url);
Expand Down Expand Up @@ -40,6 +40,7 @@ describe("flub release fromTag", () => {
.stdout()
.command(["release:fromTag", "build-tools_v0.26.1", "--json"])
.it(`--json`, (ctx) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const output: jsonOutput = JSON.parse(ctx.stdout);
// const { title, tag, version } = output;
expect(output).to.deep.equal(expected);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ describe("flub test-only-filter", () => {
.stdout()
.command(["test-only-filter", "--quiet", "--json", "--all"])
.it(`--all selector`, (ctx) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const output: jsonOutput = JSON.parse(ctx.stdout);
const { selected, filtered } = output;
expect(selected.length).to.equal(filtered.length);
Expand All @@ -31,15 +32,16 @@ describe("flub test-only-filter", () => {
.stdout()
.command(["test-only-filter", "--quiet", "--json", "--dir", "."])
.it(`--dir selector`, (ctx) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const output: jsonOutput = JSON.parse(ctx.stdout);
const { selected, filtered } = output;
expect(selected).to.be.ofSize(1);
expect(filtered).to.be.ofSize(1);

const pkg = filtered[0];

expect(pkg.name).to.equal("@fluid-tools/build-cli");
expect(pkg.directory).to.equal("build-tools/packages/build-cli");
expect(pkg?.name).to.equal("@fluid-tools/build-cli");
expect(pkg?.directory).to.equal("build-tools/packages/build-cli");

expect(selected.length).to.equal(1);
expect(filtered.length).to.equal(1);
Expand All @@ -49,6 +51,7 @@ describe("flub test-only-filter", () => {
.stdout()
.command(["test-only-filter", "--quiet", "--json", "--releaseGroup", "build-tools"])
.it(`--releaseGroup selector`, (ctx) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const output: jsonOutput = JSON.parse(ctx.stdout);
const { selected, filtered } = output;
expect(selected).to.be.ofSize(4);
Expand All @@ -59,6 +62,7 @@ describe("flub test-only-filter", () => {
.stdout()
.command(["test-only-filter", "--quiet", "--json", "--all", "--private"])
.it(`--private filter`, (ctx) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const output: jsonOutput = JSON.parse(ctx.stdout);
const { filtered } = output;

Expand All @@ -73,6 +77,7 @@ describe("flub test-only-filter", () => {
.stdout()
.command(["test-only-filter", "--quiet", "--json", "--all", "--no-private"])
.it(`--no-private filter`, (ctx) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const output: jsonOutput = JSON.parse(ctx.stdout);
const { filtered } = output;

Expand All @@ -91,6 +96,7 @@ describe("flub test-only-filter", () => {
"@fluidframework",
])
.it(`--scope filter`, (ctx) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const output: jsonOutput = JSON.parse(ctx.stdout);
const { filtered } = output;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ import {
previousVersion,
resetBrokenTests,
updateTypeTestDependency,
} from "../../src/commands/typetests.js";
} from "../../commands/typetests.js";
import {
type ITypeValidationConfig,
type PackageWithTypeTestSettings,
defaultTypeValidationConfig,
} from "../../src/typeValidator/typeValidatorConfig.js";
// eslint-disable-next-line import/no-internal-modules
} from "../../typeValidator/typeValidatorConfig.js";

/**
* A minimal test package.json. It defines only the required fields according to the type definition.
Expand Down Expand Up @@ -53,6 +54,7 @@ function packageWithTypeValidation(enabled = true): PackageWithTypeTestSettings
}

describe("typetests tests", () => {
/* eslint-disable @typescript-eslint/no-unused-expressions */
describe("updateTypeTestDependency", () => {
it("does not remove unrelated dependencies", () => {
const pkg: PackageWithTypeTestSettings = {
Expand Down Expand Up @@ -126,6 +128,7 @@ describe("typetests tests", () => {
});
});
});
/* eslint-enable @typescript-eslint/no-unused-expressions */

describe("resetBrokenTests", () => {
it("empty", () => {
Expand Down Expand Up @@ -156,6 +159,7 @@ describe("typetests tests", () => {
const pkg = packageMinimal();
const expected = packageMinimal();

// eslint-disable-next-line @typescript-eslint/no-unused-expressions
expect(pkg.typeValidation).to.not.exist;
resetBrokenTests(pkg);
expect(pkg).to.deep.equal(expected);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@
/**
* @public
*/
// eslint-disable-next-line @typescript-eslint/no-extraneous-class
export declare class Foo {}
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ export declare type A = number;
/**
* @public
*/
// eslint-disable-next-line one-var
export declare const a: number, b: string;
/**
* @internal
*/
export declare const c: number;

import * as InternalTypes from "./innerFile.js";
// eslint-disable-next-line unicorn/prefer-export-from
export { InternalTypes };
Original file line number Diff line number Diff line change
Expand Up @@ -5,46 +5,45 @@

import path from "node:path";
import { fileURLToPath } from "node:url";
import { GitRepo, getResolvedFluidRoot } from "@fluidframework/build-tools";
import { PackageName } from "@rushstack/node-core-library";
import chai, { assert, expect } from "chai";
import { GitRepo, type Package, getResolvedFluidRoot } from "@fluidframework/build-tools";
import chai, { expect } from "chai";
import assertArrays from "chai-arrays";
import {
AllPackagesSelectionCriteria,
PackageFilterOptions,
PackageSelectionCriteria,
filterPackages,
selectAndFilterPackages,
} from "../src/filter.js";
import { Context } from "../src/library/index.js";
} from "../filter.js";
import { Context } from "../library/index.js";

const __dirname = path.dirname(fileURLToPath(import.meta.url));

chai.use(assertArrays);

async function getContext() {
async function getContext(): Promise<Context> {
const resolvedRoot = await getResolvedFluidRoot();
const gitRepo = new GitRepo(resolvedRoot);
const branch = await gitRepo.getCurrentBranchName();
const context = new Context(gitRepo, "microsoft/FluidFramework", branch);
return context;
}

async function getBuildToolsPackages() {
async function getBuildToolsPackages(): Promise<Package[]> {
const context = await getContext();
// Use the build-tools packages as test cases. It's brittle, but functional. Ideally, we would have mocks for
// context/package/release group/etc., but we don't.
const packages = context.packagesInReleaseGroup("build-tools");
return packages;
}

async function getClientPackages() {
async function getClientPackages(): Promise<Package[]> {
const context = await getContext();
const packages = context.packagesInReleaseGroup("client");
return packages;
}

describe("filterPackages", async () => {
describe("filterPackages", () => {
it("no filters", async () => {
const packages = await getBuildToolsPackages();
const filters: PackageFilterOptions = {
Expand Down Expand Up @@ -137,7 +136,7 @@ describe("filterPackages", async () => {
});
});

describe("selectAndFilterPackages", async () => {
describe("selectAndFilterPackages", () => {
it("all, no filters", async () => {
const context = await getContext();
const selectionOptions = AllPackagesSelectionCriteria;
Expand Down Expand Up @@ -260,8 +259,8 @@ describe("selectAndFilterPackages", async () => {

const pkg = filtered[0];

expect(pkg.name).to.equal("@fluid-tools/build-cli");
expect(context.repo.relativeToRepo(pkg.directory)).to.equal(
expect(pkg?.name).to.equal("@fluid-tools/build-cli");
expect(context.repo.relativeToRepo(pkg?.directory ?? "")).to.equal(
"build-tools/packages/build-cli",
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@
import { test as oclifTest } from "@oclif/test";

/**
* Initializes the oclif command test environment. @oclif/test cannot find the path to the project in some
* Initializes the oclif command test environment. \@oclif/test cannot find the path to the project in some
* circumstances, so as a workaround we configure it explicitly by passing in the URL to the test module itself.
*
* @param moduleUrl - The URL to the test module. In most cases you should pass the `import.meta.url` value for the test
* module when calling this function.
*
* @returns A test function that can be used to tst oclif commands.
*/
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/explicit-function-return-type
export function initializeCommandTestFunction(moduleUrl: string) {
// @oclif/test cannot find the path to the project, so as a workaround we configure it explicitly
return oclifTest.loadConfig({ root: moduleUrl });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@

import { assert } from "chai";

import { MonoRepoKind } from "../../src/library/index.js";
import { MonoRepoKind } from "../../library/index.js";

import {
generateBumpDepsBranchName,
generateBumpVersionBranchName,
generateReleaseBranchName,
getDefaultBumpTypeForBranch,
} from "../../src/library/branches.js";
// eslint-disable-next-line import/no-internal-modules
} from "../../library/branches.js";

describe("generateBumpVersionBranchName", () => {
it("semver versions", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ import {
groupByMainPackage,
groupByPackage,
loadChangesets,
} from "../../src/library/changesets.js";
// eslint-disable-next-line import/no-internal-modules
} from "../../library/changesets.js";

const __dirname = path.dirname(fileURLToPath(import.meta.url));

const changesetsPath = path.resolve(__dirname, "../data");
assert.isTrue(existsSync(changesetsPath));

describe("changesets", async () => {
describe("changesets", () => {
it("loadChangesets", async () => {
const changesets = await loadChangesets(changesetsPath);
expect(changesets.length).to.equal(3);
Expand Down
Loading
Loading