Skip to content

Commit

Permalink
Fix errors logged in SnsProposalDetail.spec.ts
Browse files Browse the repository at this point in the history
  • Loading branch information
dskloetd committed Jan 8, 2025
1 parent ba5bba6 commit 37a52eb
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 4 deletions.
2 changes: 1 addition & 1 deletion frontend/src/tests/fakes/sns-governance-api.fake.ts
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ async function queryProposal({
}): Promise<SnsProposalData> {
const proposal = proposals
.get(mapKey({ identity, rootCanisterId }))
.proposals.find(({ id }) => fromNullable(id).id === proposalId.id);
?.proposals.find(({ id }) => fromNullable(id).id === proposalId.id);
if (isNullish(proposal)) {
throw new SnsGovernanceError(
`No proposal for given proposalId ${proposalId.id}`
Expand Down
72 changes: 70 additions & 2 deletions frontend/src/tests/lib/pages/SnsProposalDetail.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ describe("SnsProposalDetail", () => {

describe("not logged in", () => {
beforeEach(() => {
vi.spyOn(console, "error").mockImplementation(() => undefined);
setNoIdentity();
page.mock({ data: { universe: rootCanisterId.toText() } });
setSnsProjects([
Expand Down Expand Up @@ -210,6 +209,7 @@ describe("SnsProposalDetail", () => {
});

it("should redirect to the list of sns proposals if proposal is not found", async () => {
vi.spyOn(console, "error").mockReturnValue();
// There is no proposal with id 2 in the fake implementation.
// Therefore, the page should redirect to the list of proposals.
render(SnsProposalDetail, {
Expand All @@ -221,6 +221,12 @@ describe("SnsProposalDetail", () => {
const { path } = get(pageStore);
return expect(path).toEqual(AppPath.Proposals);
});
expect(console.error).toBeCalledWith(
expect.objectContaining({
error: new Error("No proposal for given proposalId 2"),
})
);
expect(console.error).toBeCalledTimes(1);
});

it("should not render content if universe changes to Nns", async () => {
Expand Down Expand Up @@ -401,6 +407,7 @@ describe("SnsProposalDetail", () => {
});

it("should navigate to the proposal from the next Sns", async () => {
vi.spyOn(console, "error").mockReturnValue();
mockCommittedSnsProjectsWithVotableProposals([
{ rootCanisterId: principal1, proposalIds: [20n, 19n] },
{ rootCanisterId: principal2, proposalIds: [30n, 29n] },
Expand Down Expand Up @@ -433,9 +440,32 @@ describe("SnsProposalDetail", () => {
id: "/(app)/proposal/",
},
});

await runResolvedPromises();
// The navigation changes both the proposal ID and the universe.
// In reality this will cause a navigation and a new component will be
// rendered with the new proposal ID and universe.
// But in the test, the component notices the change in universe (because
// it comes from the store) but not the change in proposal ID (because
// it's passed in as a prop). So it tries to load proposal 19 from the
// wrong universe, which causes these errors.
expect(console.error).toHaveBeenNthCalledWith(
2,
expect.objectContaining({
error: new Error("No proposal for given proposalId 19"),
})
);
expect(console.error).toHaveBeenNthCalledWith(
1,
expect.objectContaining({
error: new Error("No proposal for given proposalId 19"),
})
);
expect(console.error).toBeCalledTimes(2);
});

it("should navigate to the proposal from the previous Sns", async () => {
vi.spyOn(console, "error").mockReturnValue();
mockCommittedSnsProjectsWithVotableProposals([
{ rootCanisterId: principal1, proposalIds: [20n, 19n] },
{ rootCanisterId: principal2, proposalIds: [30n, 29n] },
Expand Down Expand Up @@ -468,12 +498,33 @@ describe("SnsProposalDetail", () => {
id: "/(app)/proposal/",
},
});

await runResolvedPromises();
// The navigation changes both the proposal ID and the universe.
// In reality this will cause a navigation and a new component will be
// rendered with the new proposal ID and universe.
// But in the test, the component notices the change in universe (because
// it comes from the store) but not the change in proposal ID (because
// it's passed in as a prop). So it tries to load proposal 30 from the
// wrong universe, which causes these errors.
expect(console.error).toHaveBeenNthCalledWith(
2,
expect.objectContaining({
error: new Error("No proposal for given proposalId 30"),
})
);
expect(console.error).toHaveBeenNthCalledWith(
1,
expect.objectContaining({
error: new Error("No proposal for given proposalId 30"),
})
);
expect(console.error).toBeCalledTimes(2);
});
});

describe("not logged in that logs in afterwards", () => {
beforeEach(() => {
vi.spyOn(console, "error").mockImplementation(() => undefined);
page.mock({ data: { universe: rootCanisterId.toText() } });
});

Expand Down Expand Up @@ -527,6 +578,23 @@ describe("SnsProposalDetail", () => {
],
});

fakeSnsGovernanceApi.addProposalWith({
identity: mockIdentity,
rootCanisterId,
...proposal,
proposal_creation_timestamp_seconds: proposalCreatedTimestamp,
ballots: [
[
getSnsNeuronIdAsHexString(mockSnsNeuron),
{
vote: SnsVote.Unspecified,
voting_power: 100_000_000n,
cast_timestamp_seconds: 0n,
},
],
],
});

const { container } = render(SnsProposalDetail, {
props: {
proposalIdText: proposalId.id.toString(),
Expand Down
5 changes: 4 additions & 1 deletion frontend/src/tests/utils/console.test-utils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { runResolvedPromises } from "$tests/utils/timers.test-utils";

type LogType = "log" | "debug" | "warn" | "error";

const logTypes: LogType[] = ["log", "debug", "warn", "error"];
Expand All @@ -22,7 +24,8 @@ export const failTestsThatLogToConsole = () => {
gotLogs = false;
});

afterEach(() => {
afterEach(async () => {
await runResolvedPromises();
if (!isLoggingAllowed && gotLogs) {
throw new Error(
"Your test produced console logs, which is not allowed.\n" +
Expand Down

0 comments on commit 37a52eb

Please sign in to comment.