factor loadWeights into Common (#1377)

As suggested by @Beanow in [a review comment][1], this commit factors
loading weights from disk into a cli/common utility method.

The actual method is really generic, and we have a number of similar
constructions across the codebase (grep for `JSON.parse` to find them).
I considered factoring out a generic utility for loading and
deserializing JSON data from disk in general, but it didn't seem
valuable enough at this time.

Test plan: Unit tests added, existing tests pass.

[1]: https://github.com/sourcecred/sourcecred/pull/1374#discussion_r323149740
This commit is contained in:
Dandelion Mané 2019-09-12 15:55:05 +02:00 committed by GitHub
parent 0a0010f38e
commit 7a0dd49b42
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 54 additions and 17 deletions

View File

@ -4,6 +4,8 @@
import os from "os"; import os from "os";
import path from "path"; import path from "path";
import deepFreeze from "deep-freeze"; import deepFreeze from "deep-freeze";
import fs from "fs-extra";
import {type Weights, fromJSON as weightsFromJSON} from "../analysis/weights";
import * as NullUtil from "../util/null"; import * as NullUtil from "../util/null";
@ -27,3 +29,16 @@ export function githubToken(): string | null {
export function discourseKey(): string | null { export function discourseKey(): string | null {
return NullUtil.orElse(process.env.SOURCECRED_DISCOURSE_KEY, null); return NullUtil.orElse(process.env.SOURCECRED_DISCOURSE_KEY, null);
} }
export async function loadWeights(path: string): Promise<Weights> {
if (!(await fs.exists(path))) {
throw new Error("Could not find the weights file");
}
const raw = await fs.readFile(path, "utf-8");
const weightsJSON = JSON.parse(raw);
try {
return weightsFromJSON(weightsJSON);
} catch (e) {
throw new Error(`provided weights file is invalid:\n${e}`);
}
}

View File

@ -1,6 +1,10 @@
// @flow // @flow
import path from "path"; import path from "path";
import tmp from "tmp";
import fs from "fs-extra";
import {defaultWeights, toJSON as weightsToJSON} from "../analysis/weights";
import {NodeAddress} from "../core/graph";
import { import {
defaultPlugins, defaultPlugins,
@ -8,6 +12,7 @@ import {
sourcecredDirectory, sourcecredDirectory,
githubToken, githubToken,
discourseKey, discourseKey,
loadWeights,
} from "./common"; } from "./common";
describe("cli/common", () => { describe("cli/common", () => {
@ -66,4 +71,36 @@ describe("cli/common", () => {
expect(discourseKey()).toBe(null); expect(discourseKey()).toBe(null);
}); });
}); });
describe("loadWeights", () => {
function tmpWithContents(contents: mixed) {
const name = tmp.tmpNameSync();
fs.writeFileSync(name, JSON.stringify(contents));
return name;
}
it("works in a simple success case", async () => {
const weights = defaultWeights();
// Make a modification, just to be sure we aren't always loading the
// default weights.
weights.nodeManualWeights.set(NodeAddress.empty, 3);
const weightsJSON = weightsToJSON(weights);
const file = tmpWithContents(weightsJSON);
const weights_ = await loadWeights(file);
expect(weights).toEqual(weights_);
});
it("rejects if the file is not a valid weights file", () => {
const file = tmpWithContents(1234);
expect.assertions(1);
return loadWeights(file).catch((e) =>
expect(e.message).toMatch("provided weights file is invalid:")
);
});
it("rejects if the file does not exist", () => {
const file = tmp.tmpNameSync();
expect.assertions(1);
return loadWeights(file).catch((e) =>
expect(e.message).toMatch("Could not find the weights file")
);
});
});
}); });

View File

@ -5,10 +5,9 @@ import dedent from "../util/dedent";
import {LoggingTaskReporter} from "../util/taskReporter"; import {LoggingTaskReporter} from "../util/taskReporter";
import type {Command} from "./command"; import type {Command} from "./command";
import * as Common from "./common"; import * as Common from "./common";
import {defaultWeights, fromJSON as weightsFromJSON} from "../analysis/weights"; import {defaultWeights} from "../analysis/weights";
import {load} from "../api/load"; import {load} from "../api/load";
import {specToProject} from "../plugins/github/specToProject"; import {specToProject} from "../plugins/github/specToProject";
import fs from "fs-extra";
import {partialParams} from "../analysis/timeline/params"; import {partialParams} from "../analysis/timeline/params";
import {DEFAULT_PLUGINS} from "./defaultPlugins"; import {DEFAULT_PLUGINS} from "./defaultPlugins";
@ -93,7 +92,7 @@ const loadCommand: Command = async (args, std) => {
let weights = defaultWeights(); let weights = defaultWeights();
if (weightsPath) { if (weightsPath) {
weights = await loadWeightOverrides(weightsPath); weights = await Common.loadWeights(weightsPath);
} }
const githubToken = Common.githubToken(); const githubToken = Common.githubToken();
@ -124,20 +123,6 @@ const loadCommand: Command = async (args, std) => {
return 0; return 0;
}; };
const loadWeightOverrides = async (path: string) => {
if (!(await fs.exists(path))) {
throw new Error("Could not find the weights file");
}
const raw = await fs.readFile(path, "utf-8");
const weightsJSON = JSON.parse(raw);
try {
return weightsFromJSON(weightsJSON);
} catch (e) {
throw new Error(`provided weights file is invalid:\n${e}`);
}
};
export const help: Command = async (args, std) => { export const help: Command = async (args, std) => {
if (args.length === 0) { if (args.length === 0) {
usage(std.out); usage(std.out);