Skip to content

Commit

Permalink
refactor: misc improvements in tests (#60)
Browse files Browse the repository at this point in the history
* chore: add fast-check dependency

* refactor: type add-cmd result

Add type information indicating that result code will always be either 0 or 1

* refactor: adjust typings

Use information found in test usage to adjust types throughout the project. Also type constants in tests

* refactor: add mock pkg registry for tests

Add utility functions to de-duplicate the common logic of adding packages and search results to the mock package registry in tests

* refactor: reuse constant

* refactor: split file

Split utilities file into mock-work-dir and mock-console

* refactor: add type-alias

Hide implementation detail of what mock console is used from tests

* refactor: deduplicate test code

Add helpers  to check for correct manifest in add cmd tests

* refactor: deduplicate logic  in tests

Move logic for, creating, resetting and querying mock console content into dedicated functions

* deps: bump packages

To avoid warnings. Only dev dependencies

* refactor: remove unnecessary await

* refactor: extract helpers

Move manifest assertion helpers to own module so they can be reused

* deps: remove stub type package

* refactor: manifest assertion

prepare for allowing to chain multiple assertions on same manifest

* refactor: deduplicate code

Use manifest assertions in cmd remove tests. Also add a few new ones

* refactor: deduplicate code

Use manifest assertions in manifest tests

* refactor: use better type for pkg-info name

There should never be a version inside the name

* deps: remove fast check

* refactor: optimize imports

* refactor: remove duplicate code

* refactor: simplify defined assertions
  • Loading branch information
ComradeVanti authored Nov 20, 2023
1 parent 5d9a0d5 commit 3abf506
Show file tree
Hide file tree
Showing 22 changed files with 7,661 additions and 719 deletions.
7,280 changes: 7,104 additions & 176 deletions package-lock.json

Large diffs are not rendered by default.

5 changes: 2 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
},
"homepage": "https://openupm.com",
"devDependencies": {
"@babel/core": "^7.17.10",
"@babel/core": "^7.18.6",
"@babel/eslint-parser": "^7.17.0",
"@semantic-release/changelog": "^6.0.1",
"@semantic-release/git": "^10.0.1",
Expand All @@ -47,7 +47,6 @@
"@types/pkginfo": "^0.4.1",
"@types/promptly": "^3.0.3",
"@types/request": "^2.48.11",
"@types/should": "^13.0.0",
"@types/test-console": "^2.0.1",
"@types/update-notifier": "^6.0.5",
"@typescript-eslint/eslint-plugin": "^6.8.0",
Expand All @@ -57,7 +56,7 @@
"eslint-config-prettier": "^8.5.0",
"eslint-plugin-prettier": "^4.0.0",
"eslint-plugin-vue": "^8.7.1",
"mocha": "^10.0.0",
"mocha": "^10.1.0",
"nock": "^13.2.4",
"prettier": "^2.6.2",
"rimraf": "^5.0.5",
Expand Down
13 changes: 10 additions & 3 deletions src/cmd-add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,17 @@ export type AddOptions = {
_global: GlobalOptions;
};

type ResultCode = 0 | 1;

type AddResult = {
dirty: boolean;
code: ResultCode;
};

export const add = async function (
pkgs: PkgName | PkgName[],
options: AddOptions
): Promise<number> {
): Promise<ResultCode> {
if (!Array.isArray(pkgs)) pkgs = [pkgs];
// parse env
const envOk = await parseEnv(options, { checkPath: true });
Expand All @@ -33,7 +40,7 @@ export const add = async function (
results.push(
await _add({ pkg, testables: options.test, force: options.force })
);
const result = {
const result: AddResult = {
code: results.filter((x) => x.code != 0).length > 0 ? 1 : 0,
dirty: results.filter((x) => x.dirty).length > 0,
};
Expand All @@ -51,7 +58,7 @@ const _add = async function ({
pkg: PkgName;
testables?: boolean;
force?: boolean;
}) {
}): Promise<AddResult> {
// dirty flag
let dirty = false;
// is upstream package flag
Expand Down
20 changes: 16 additions & 4 deletions src/cmd-search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
PkgName,
PkgVersion,
Registry,
ReverseDomainName,
} from "./types/global";
import { tryGetLatestVersion } from "./utils/pkg-info";
import { env, parseEnv } from "./utils/env";
Expand All @@ -22,6 +23,15 @@ type TableRow = [PkgName, PkgVersion, DateString, ""];
export type SearchOptions = {
_global: GlobalOptions;
};

export type SearchedPkgInfo = Omit<PkgInfo, "versions"> & {
versions: Record<PkgVersion, "latest">;
};

export type OldSearchResult =
| SearchedPkgInfo[]
| Record<ReverseDomainName, SearchedPkgInfo>;

// Get npm fetch options
const getNpmFetchOptions = function (): Options {
const opts: Options = {
Expand All @@ -40,7 +50,9 @@ const searchEndpoint = async function (
if (!registry) registry = env.registry;
try {
// NOTE: The results of the search will be PkgInfo objects so we can change the type
const results = <PkgInfo[]>await npmSearch(keyword, getNpmFetchOptions());
const results = <SearchedPkgInfo[]>(
await npmSearch(keyword, getNpmFetchOptions())
);
log.verbose("npmsearch", results.join(os.EOL));
return results.map(getTableRow);
} catch (err) {
Expand All @@ -56,10 +68,10 @@ const searchOld = async function (
): Promise<TableRow[] | undefined> {
// all endpoint
try {
const results = <Record<string, PkgInfo> | PkgInfo[]>(
const results = <OldSearchResult | undefined>(
await npmFetch.json("/-/all", getNpmFetchOptions())
);
let objects: PkgInfo[] = [];
let objects: SearchedPkgInfo[] = [];
if (results) {
if (Array.isArray(results)) {
// results is an array of objects
Expand Down Expand Up @@ -95,7 +107,7 @@ const getTable = function () {
});
};

const getTableRow = function (pkg: PkgInfo): TableRow {
const getTableRow = function (pkg: SearchedPkgInfo): TableRow {
const name = pkg.name;
const version = tryGetLatestVersion(pkg);
let date = "";
Expand Down
2 changes: 1 addition & 1 deletion src/cmd-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const printInfo = function (pkg: PkgInfo) {
const homepage = verInfo.homepage;
const dist = verInfo.dist;
const dependencies = verInfo.dependencies;
const latest = pkg["dist-tags"].latest;
const latest = pkg["dist-tags"]?.latest;
let time = pkg.time.modified;
if (!time && latest && latest in pkg.time) time = pkg.time[latest];

Expand Down
34 changes: 28 additions & 6 deletions src/types/global.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,27 +44,49 @@ export type Dist = {
integrity: string;
};

export type Contact = {
name: string;
email?: string;
url?: string;
};

export type PkgVersionInfo = {
_id?: PkgName;
_nodeVersion?: string;
_npmVersion?: string;
_rev?: string;
name: string;
version: string;
unity?: string;
unityRelease: string;
dependencies: Record<PkgName, PkgVersion>;
unityRelease?: string;
dependencies?: Record<PkgName, PkgVersion>;
license?: string;
displayName: string;
displayName?: string;
description?: string;
keywords?: string[];
homepage: string;
homepage?: string;
category?: string;
gitHead?: string;
readmeFilename?: string;
author?: Contact;
contributors?: Contact[];
dist?: Dist;
};

export type PkgInfo = {
name: PkgName;
name: ReverseDomainName;
_id?: PkgName;
_rev?: string;
_attachments?: Record<string, unknown>;
readme?: string;
versions: Record<PkgVersion, PkgVersionInfo>;
"dist-tags": { latest?: PkgVersion };
"dist-tags"?: { latest?: PkgVersion };
version?: PkgVersion;
description?: string;
keywords?: string[];
time: Record<"created" | "modified" | PkgVersion, string>;
date?: Date;
users?: Record<string, unknown>;
};

export type NameVersionPair = {
Expand Down
7 changes: 4 additions & 3 deletions src/utils/pkg-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ const hasLatestDistTag = (
* Attempt to get the latest version from a package
* @param pkgInfo The package. All properties are assumed to be potentially missing
*/
export const tryGetLatestVersion = function (
pkgInfo: Partial<PkgInfo>
): PkgVersion | undefined {
export const tryGetLatestVersion = function (pkgInfo: {
"dist-tags"?: { latest?: PkgVersion };
version?: PkgVersion;
}): PkgVersion | undefined {
if (hasLatestDistTag(pkgInfo)) return pkgInfo["dist-tags"].latest;
else if (pkgInfo.version) return pkgInfo.version;
};
41 changes: 41 additions & 0 deletions test/manifest-assertions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { PkgManifest, PkgName, PkgVersion } from "../src/types/global";
import { loadManifest } from "../src/utils/manifest";
import should from "should";

export function shouldHaveManifest(): PkgManifest {
const manifest = loadManifest();
should(manifest).not.be.null();
return manifest!;
}

export function shouldHaveNoManifest() {
const manifest = loadManifest();
should(manifest).be.null();
}

export function shouldHaveDependency(
manifest: PkgManifest,
name: PkgName,
version: PkgVersion
) {
should(manifest.dependencies[name]).equal(version);
}

export function shouldNotHaveAnyDependencies(manifest: PkgManifest) {
should(manifest.dependencies).be.empty();
}
export function shouldNotHaveDependency(manifest: PkgManifest, name: PkgName) {
should(manifest.dependencies[name]).be.undefined();
}

export function shouldHaveRegistryWithScopes(
manifest: PkgManifest,
scopes: PkgName[]
) {
should(manifest.scopedRegistries).not.be.undefined();
manifest
.scopedRegistries!.some((registry) =>
scopes.every((scope) => registry.scopes.includes(scope))
)
.should.be.true("At least one scope was missing");
}
23 changes: 23 additions & 0 deletions test/mock-console.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import testConsole from "test-console";

type Stream = "out" | "error";

export type MockConsole = {
hasLineIncluding(stream: Stream, text: string): boolean;
detach(): void;
};

export function attachMockConsole(): MockConsole {
const out = testConsole.stdout.inspect();
const error = testConsole.stderr.inspect();
return {
hasLineIncluding(stream: Stream, text: string): boolean {
const inspector = stream === "out" ? out : error;
return inspector.output.some((line) => line.includes(text));
},
detach() {
out.restore();
error.restore();
},
};
}
45 changes: 45 additions & 0 deletions test/mock-registry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { PkgInfo, PkgName } from "../src/types/global";
import nock from "nock";
import { SearchEndpointResult } from "./types";

export const unityRegistryUrl = "https://packages.unity.com";
export const exampleRegistryUrl = "http://example.com";

export function startMockRegistry() {
if (!nock.isActive()) nock.activate();
}

export function registerRemotePkg(pkg: PkgInfo) {
nock(exampleRegistryUrl)
.persist()
.get(`/${pkg.name}`)
.reply(200, pkg, { "Content-Type": "application/json" });
}

export function registerRemoteUpstreamPkg(pkg: PkgInfo) {
nock(unityRegistryUrl).persist().get(`/${pkg.name}`).reply(200, pkg, {
"Content-Type": "application/json",
});
nock(exampleRegistryUrl).persist().get(`/${pkg.name}`).reply(404);
}

export function registerMissingPackage(name: PkgName) {
nock(exampleRegistryUrl).persist().get(`/${name}`).reply(404);
nock(unityRegistryUrl).persist().get(`/${name}`).reply(404);
}

export function registerSearchResult(
searchText: string,
result: SearchEndpointResult
) {
nock(exampleRegistryUrl)
.get(new RegExp(`-\\/v1\\/search\\?text=${searchText}`))
.reply(200, result, {
"Content-Type": "application/json",
});
}

export function stopMockRegistry() {
nock.restore();
nock.cleanAll();
}
38 changes: 1 addition & 37 deletions test/utils.ts → test/mock-work-dir.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,16 @@
import fse from "fs-extra";
import nock from "nock";
import path from "path";
import os from "os";
import testConsole from "test-console";
import { PkgManifest } from "../src/types/global";
import fse from "fs-extra";
import _ from "lodash";

export type ManifestCreationOptions = {
manifest: boolean | PkgManifest;
editorVersion?: string;
};

export const getWorkDir = function (pathToTmp: string): string {
return path.join(os.tmpdir(), pathToTmp);
};

export const createWorkDir = function (
pathToTmp: string,
{ manifest, editorVersion }: ManifestCreationOptions
Expand All @@ -38,39 +34,7 @@ export const createWorkDir = function (
);
}
};

export const removeWorkDir = function (pathToTmp: string) {
const cwd = getWorkDir(pathToTmp);
fse.removeSync(cwd);
};

export const nockUp = function () {
if (!nock.isActive()) nock.activate();
};

export const nockDown = function () {
nock.restore();
nock.cleanAll();
};

export const getOutputs = function (
stdouInspect: testConsole.Inspector,
stderrInsepct: testConsole.Inspector
): [string, string] {
const results: [string, string] = [
stdouInspect.output.join(""),
stderrInsepct.output.join(""),
];
stdouInspect.restore();
stderrInsepct.restore();
return results;
};

export const getInspects = function (): [
testConsole.Inspector,
testConsole.Inspector
] {
const stdoutInspect = testConsole.stdout.inspect();
const stderrInspect = testConsole.stderr.inspect();
return [stdoutInspect, stderrInspect];
};
Loading

0 comments on commit 3abf506

Please sign in to comment.