Skip to content

Commit

Permalink
feat(publish): Use APIs for validation queries instead of CLI
Browse files Browse the repository at this point in the history
This removes the `--no-verify-registry` flag, as the former code path is no longer called, having been replaced with a far more robust login validation.
  • Loading branch information
evocateur committed Sep 14, 2018
1 parent 21aa430 commit 65fc603
Show file tree
Hide file tree
Showing 32 changed files with 319 additions and 337 deletions.
7 changes: 1 addition & 6 deletions commands/publish/README.md
Expand Up @@ -44,7 +44,6 @@ but have the package contents themselves consistently published by an automated
* [`--npm-client <client>`](#--npm-client-client)
* [`--npm-tag <dist-tag>`](#--npm-tag-dist-tag)
* [`--no-verify-access`](#--no-verify-access)
* [`--no-verify-registry`](#--no-verify-registry)
* [`--registry <url>`](#--registry-url)
* [`--temp-tag`](#--temp-tag)
* [`--yes`](#--yes)
Expand Down Expand Up @@ -107,11 +106,7 @@ This option can be used to publish a [`prerelease`](http://carrot.is/coding/npm_

By default, `lerna` will verify the logged-in npm user's access to the packages about to be published. Passing this flag will disable that check.

> Please use with caution
### `--no-verify-registry`

By default, `lerna` will verify that the npm registry is reachable and the current npm user is authenticated. Passing this flag will disable that check.
If you are using a third-party registry that does not support `npm access ls-packages`, you will need to pass this flag (or set `command.publish.verifyAccess` to `false` in lerna.json).

> Please use with caution
Expand Down
63 changes: 63 additions & 0 deletions commands/publish/__tests__/get-npm-username.test.js
@@ -0,0 +1,63 @@
"use strict";

jest.mock("npm-registry-fetch");

const fetch = require("npm-registry-fetch");
const getNpmUsername = require("../lib/get-npm-username");

fetch.json.mockImplementation(() => Promise.resolve({ username: "lerna-test" }));

describe("getNpmUsername", () => {
const origConsoleError = console.error;

beforeEach(() => {
console.error = jest.fn();
});

afterEach(() => {
console.error = origConsoleError;
});

test("fetches whoami endpoint", async () => {
const opts = { such: "npm-conf", wow: true };

const username = await getNpmUsername(opts);

expect(username).toBe("lerna-test");
expect(fetch.json).lastCalledWith("-/whoami", opts);
});

test("throws an error when successful fetch yields empty username", async () => {
fetch.json.mockImplementationOnce(() => Promise.resolve({ username: undefined }));

try {
await getNpmUsername({ stub: true });
} catch (err) {
expect(err.prefix).toBe("ENEEDAUTH");
expect(err.message).toBe("You must be logged in to publish packages. Use `npm login` and try again.");
expect(console.error).not.toBeCalled();
}

expect.assertions(3);
});

test("logs failure message before throwing validation error", async () => {
fetch.json.mockImplementationOnce(() => {
const err = new Error("whoops");

err.code = "E500";

return Promise.reject(err);
});

try {
await getNpmUsername({ stub: true });
} catch (err) {
expect(err.prefix).toBe("EWHOAMI");
expect(err.message).toBe("Authentication error. Use `npm whoami` to troubleshoot.");
expect(console.error).toBeCalledWith("whoops");
}

expect.assertions(3);
});
});
2 changes: 1 addition & 1 deletion commands/publish/__tests__/publish-canary.test.js
Expand Up @@ -6,7 +6,7 @@ jest.unmock("@lerna/collect-updates");
// local modules _must_ be explicitly mocked
jest.mock("../lib/get-packages-without-license");
jest.mock("../lib/verify-npm-package-access");
jest.mock("../lib/verify-npm-registry");
jest.mock("../lib/get-npm-username");

const fs = require("fs-extra");
const path = require("path");
Expand Down
24 changes: 7 additions & 17 deletions commands/publish/__tests__/publish-command.test.js
Expand Up @@ -3,7 +3,7 @@
// local modules _must_ be explicitly mocked
jest.mock("../lib/get-packages-without-license");
jest.mock("../lib/verify-npm-package-access");
jest.mock("../lib/verify-npm-registry");
jest.mock("../lib/get-npm-username");
// FIXME: better mock for version command
jest.mock("../../version/lib/git-push");
jest.mock("../../version/lib/is-anything-committed");
Expand All @@ -16,7 +16,7 @@ const PromptUtilities = require("@lerna/prompt");
const collectUpdates = require("@lerna/collect-updates");
const output = require("@lerna/output");
const checkWorkingTree = require("@lerna/check-working-tree");
const verifyNpmRegistry = require("../lib/verify-npm-registry");
const getNpmUsername = require("../lib/get-npm-username");
const verifyNpmPackageAccess = require("../lib/verify-npm-package-access");

// helpers
Expand All @@ -42,7 +42,7 @@ describe("PublishCommand", () => {

const logMessages = loggingOutput("success");
expect(logMessages).toContain("No changed packages to publish");
expect(verifyNpmRegistry).not.toBeCalled();
expect(verifyNpmPackageAccess).not.toBeCalled();
});

it("exits early when no changes found from-git", async () => {
Expand All @@ -52,7 +52,7 @@ describe("PublishCommand", () => {

const logMessages = loggingOutput("success");
expect(logMessages).toContain("No changed packages to publish");
expect(verifyNpmRegistry).not.toBeCalled();
expect(verifyNpmPackageAccess).not.toBeCalled();
});

it("exits non-zero with --scope", async () => {
Expand Down Expand Up @@ -104,8 +104,8 @@ Set {
expect(npmDistTag.remove).not.toBeCalled();
expect(npmDistTag.add).not.toBeCalled();

expect(verifyNpmRegistry).toBeCalled();
expect(verifyNpmRegistry.registry.get(testDir)).toBe("default registry");
expect(getNpmUsername).toBeCalled();
expect(getNpmUsername.registry.get(testDir).get("registry")).toBe("https://registry.npmjs.org/");

expect(verifyNpmPackageAccess).toBeCalled();
expect(verifyNpmPackageAccess.registry.get(testDir)).toMatchInlineSnapshot(`
Expand All @@ -114,7 +114,7 @@ Set {
"package-2",
"package-3",
"package-4",
"registry: default",
"username: lerna-test",
}
`);
});
Expand Down Expand Up @@ -253,16 +253,6 @@ Set {
});
});

describe("--no-verify-registry", () => {
it("skips npm registry ping", async () => {
const cwd = await initFixture("normal");

await lernaPublish(cwd)("--no-verify-registry");

expect(verifyNpmRegistry).not.toBeCalled();
});
});

describe("--no-verify-access", () => {
it("skips package access verification", async () => {
const cwd = await initFixture("normal");
Expand Down
2 changes: 1 addition & 1 deletion commands/publish/__tests__/publish-licenses.test.js
Expand Up @@ -2,7 +2,7 @@

// local modules _must_ be explicitly mocked
jest.mock("../lib/verify-npm-package-access");
jest.mock("../lib/verify-npm-registry");
jest.mock("../lib/get-npm-username");
jest.mock("../lib/create-temp-licenses", () => jest.fn(() => Promise.resolve()));
jest.mock("../lib/remove-temp-licenses", () => jest.fn(() => Promise.resolve()));
// FIXME: better mock for version command
Expand Down
Expand Up @@ -3,7 +3,7 @@
// local modules _must_ be explicitly mocked
jest.mock("../lib/get-packages-without-license");
jest.mock("../lib/verify-npm-package-access");
jest.mock("../lib/verify-npm-registry");
jest.mock("../lib/get-npm-username");
// FIXME: better mock for version command
jest.mock("../../version/lib/git-push");
jest.mock("../../version/lib/is-anything-committed");
Expand Down
Expand Up @@ -6,7 +6,7 @@ jest.unmock("@lerna/collect-updates");
// local modules _must_ be explicitly mocked
jest.mock("../lib/get-packages-without-license");
jest.mock("../lib/verify-npm-package-access");
jest.mock("../lib/verify-npm-registry");
jest.mock("../lib/get-npm-username");
// FIXME: better mock for version command
jest.mock("../../version/lib/git-push");
jest.mock("../../version/lib/is-anything-committed");
Expand Down
2 changes: 1 addition & 1 deletion commands/publish/__tests__/publish-tagging.test.js
Expand Up @@ -3,7 +3,7 @@
// local modules _must_ be explicitly mocked
jest.mock("../lib/get-packages-without-license");
jest.mock("../lib/verify-npm-package-access");
jest.mock("../lib/verify-npm-registry");
jest.mock("../lib/get-npm-username");
// FIXME: better mock for version command
jest.mock("../../version/lib/git-push");
jest.mock("../../version/lib/is-anything-committed");
Expand Down

0 comments on commit 65fc603

Please sign in to comment.