From 0b0815eb1282ba223e3d3f499adadcd706ce5909 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dandelion=20Man=C3=A9?= Date: Fri, 27 Jul 2018 15:42:18 -0700 Subject: [PATCH] When repo registry fails, error to console (#544) Some slight changes were needed to the other test code to avoid spurious errors. Specifically, we now always set up a mocked fetch response in non-failure cases, even if we don't wait for it to resolve. Test plan: I manually tested it, also see the new unit tests. --- src/app/credExplorer/RepositorySelect.js | 7 ++++++- src/app/credExplorer/RepositorySelect.test.js | 21 +++++++++++++++---- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/app/credExplorer/RepositorySelect.js b/src/app/credExplorer/RepositorySelect.js index 792c628..60fc3e4 100644 --- a/src/app/credExplorer/RepositorySelect.js +++ b/src/app/credExplorer/RepositorySelect.js @@ -90,6 +90,7 @@ export async function loadStatus(localStore: LocalStore): Promise { try { const response = await fetch(REPO_REGISTRY_API); if (!response.ok) { + console.error(response); return {type: "FAILURE"}; } const json = await response.json(); @@ -105,6 +106,7 @@ export async function loadStatus(localStore: LocalStore): Promise { const sortedRepos = sortBy(availableRepos, (r) => r.owner, (r) => r.name); return {type: "VALID", availableRepos: sortedRepos, selectedRepo}; } catch (e) { + console.error(e); return {type: "FAILURE"}; } } @@ -178,7 +180,10 @@ export class PureRepositorySelect extends React.PureComponent< case "NO_REPOS": return this.renderError("Error: No repositories found."); case "FAILURE": - return this.renderError("Error: Unable to load repository registry."); + return this.renderError( + "Error: Unable to load repository registry. " + + "See console for details." + ); default: throw new Error((status.type: empty)); } diff --git a/src/app/credExplorer/RepositorySelect.test.js b/src/app/credExplorer/RepositorySelect.test.js index 71cae46..3bc1b75 100644 --- a/src/app/credExplorer/RepositorySelect.test.js +++ b/src/app/credExplorer/RepositorySelect.test.js @@ -58,7 +58,9 @@ describe("app/credExplorer/RepositorySelect", () => { ); const span = e.find("span"); - expect(span.text()).toBe("Error: Unable to load repository registry."); + expect(span.text()).toBe( + "Error: Unable to load repository registry. See console for details." + ); }); it("renders a select with all available repos as options", () => { const availableRepos = [ @@ -147,16 +149,22 @@ describe("app/credExplorer/RepositorySelect", () => { }); it("returns FAILURE on invalid fetch response", () => { fetch.mockResponseOnce(JSON.stringify(["hello"])); - expect.assertions(3); + expect.assertions(4); return loadStatus(testLocalStore()).then((status) => { expect(status).toEqual({type: "FAILURE"}); + expect(console.error).toHaveBeenCalledTimes(1); + // $ExpectFlowError + console.error = jest.fn(); }); }); it("returns FAILURE on fetch failure", () => { fetch.mockReject(new Error("some failure")); - expect.assertions(3); + expect.assertions(4); return loadStatus(testLocalStore()).then((status) => { expect(status).toEqual({type: "FAILURE"}); + expect(console.error).toHaveBeenCalledTimes(1); + // $ExpectFlowError + console.error = jest.fn(); }); }); it("loads selectedRepo from localStore, if available", () => { @@ -258,6 +266,7 @@ describe("app/credExplorer/RepositorySelect", () => { describe("RepositorySelect", () => { it("initially renders a LocalStoreRepositorySelect with status LOADING", () => { + mockRegistry([{owner: "irrelevant", name: "unused"}]); const e = shallow( ); @@ -319,15 +328,19 @@ describe("app/credExplorer/RepositorySelect", () => { ); await waitForUpdate(e); expect(onChange).toHaveBeenCalledTimes(0); + expect(console.error).toHaveBeenCalledTimes(1); + // $ExpectFlowError + console.error = jest.fn(); }); it("child onChange triggers parent onChange", () => { const onChange = jest.fn(); + const repo = {owner: "foo", name: "bar"}; + mockRegistry([repo]); const e = mount( ); const child = e.find(PureRepositorySelect); - const repo = {owner: "foo", name: "bar"}; child.props().onChange(repo); expect(onChange).toHaveBeenCalledTimes(1); expect(onChange).toHaveBeenCalledWith(repo);