Skip to content

Commit

Permalink
✨feat(llm/lld): avoid watchloop when ff is off (#7601)
Browse files Browse the repository at this point in the history
* ✨feat(llm/lld): avoid watchloop when ff is off

* 🧪feat(lld/llm): add unit test and jest config

* 🧪feat(lld/llm): useRef instead of useState to avoid loop
  • Loading branch information
LucasWerey committed Aug 20, 2024
1 parent 2871edd commit d75616a
Show file tree
Hide file tree
Showing 20 changed files with 325 additions and 116 deletions.
6 changes: 6 additions & 0 deletions .changeset/.changeset/poor-gifts-grin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"ledger-live-desktop": patch
"live-mobile": patch
---

Avoid running watchloop when LS ff is disabled
2 changes: 2 additions & 0 deletions apps/ledger-live-desktop/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const testPathIgnorePatterns = [
".yalc",
"cli/",
"test-helpers/",
"src/.*/shared\\.(ts|tsx)$",
];

const moduleNameMapper = {
Expand Down Expand Up @@ -76,6 +77,7 @@ module.exports = {
...testPathIgnorePatterns,
"(/__tests__/.*|(\\.|/)react\\.test|spec)\\.tsx",
],
testMatch: ["**/src/**/*.test.(ts|tsx)"],
},
{
...commonConfig,
Expand Down
2 changes: 1 addition & 1 deletion apps/ledger-live-desktop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"pre-build": "node ./tools/dist --pre -v",
"release": "node ./tools/dist --release -v",
"test": "pnpm test:jest:coverage && pnpm test:playwright",
"test:jest": "jest src --maxWorkers=50%",
"test:jest": "jest --maxWorkers=50%",
"test:jest:coverage": "pnpm test:jest --ci --coverage",
"test:playwright:setup": "playwright install chromium",
"test:playwright": "playwright test --config=tests/playwright.config.ts --project=mocked_tests",
Expand Down
37 changes: 20 additions & 17 deletions apps/ledger-live-desktop/scripts/resolver.js
Original file line number Diff line number Diff line change
@@ -1,29 +1,32 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

module.exports = (path, options) => {
// Call the defaultResolver, so we leverage its cache, error handling, etc.
return options.defaultResolver(path, {
...options,
// Use packageFilter to process parsed `package.json` before the resolution (see https://www.npmjs.com/package/resolve#resolveid-opts-cb)
packageFilter: pkg => {
// This is a workaround for https://github.com/uuidjs/uuid/pull/616
//
// jest-environment-jsdom 28+ tries to use browser exports instead of default exports,
// but uuid only offers an ESM browser export and not a CommonJS one. Jest does not yet
// support ESM modules natively, so this causes a Jest error related to trying to parse
// "export" syntax.
//
// This workaround prevents Jest from considering uuid's module-based exports at all;
// it falls back to uuid's CommonJS+node "main" property.
//
// Once we're able to migrate our Jest config to ESM and a browser crypto
// implementation is available for the browser+ESM version of uuid to use (eg, via
// https://github.com/jsdom/jsdom/pull/3352 or a similar polyfill), this can go away.
if (pkg.name === "uuid") {
const pkgNamesToTarget = new Set([
"rxjs",
"@firebase/auth",
"@firebase/storage",
"@firebase/functions",
"@firebase/database",
"@firebase/auth-compat",
"@firebase/database-compat",
"@firebase/app-compat",
"@firebase/firestore",
"@firebase/firestore-compat",
"@firebase/messaging",
"@firebase/util",
"firebase",
"uuid",
]);

if (pkgNamesToTarget.has(pkg.name)) {
// console.log('>>>', pkg.name)
delete pkg["exports"];
delete pkg["module"];
}

return pkg;
},
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,29 +1,7 @@
import React from "react";
import { render, screen, waitFor } from "tests/testUtils";
import WalletSyncRow from "~/renderer/screens/settings/sections/General/WalletSync";

import { TrustchainMember } from "@ledgerhq/trustchain/types";
import { mockedSdk, simpleTrustChain, walletSyncActivatedState } from "../testHelper/helper";

const WalletSyncTestApp = () => (
<>
<div id="modals"></div>
<WalletSyncRow />
</>
);

const INSTANCES: Array<TrustchainMember> = [
{
id: "currentInstance",
name: "macOS",
permissions: 112,
},
{
id: "2",
name: "Ipone 15",
permissions: 112,
},
];
import { WalletSyncTestApp, mockedSdk, simpleTrustChain, walletSyncActivatedState } from "./shared";
import { INSTANCES } from "./shared";

jest.mock("../hooks/useTrustchainSdk", () => ({
useTrustchainSdk: () => ({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,8 @@
import React from "react";
import { render, screen, waitFor } from "tests/testUtils";
import WalletSyncRow from "~/renderer/screens/settings/sections/General/WalletSync";
import { WalletSyncTestApp, mockedSdk, simpleTrustChain, walletSyncActivatedState } from "./shared";

import * as ReactQuery from "@tanstack/react-query";
import { mockedSdk, simpleTrustChain, walletSyncActivatedState } from "../testHelper/helper";

const WalletSyncTestApp = () => (
<>
<div id="modals"></div>
<WalletSyncRow />
</>
);

jest.mock("../hooks/useTrustchainSdk", () => ({
useTrustchainSdk: () => ({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import React from "react";
import { TrustchainMember } from "@ledgerhq/trustchain/types";
import WalletSyncRow from "~/renderer/screens/settings/sections/General/WalletSync";
import { getSdk } from "@ledgerhq/trustchain/index";
import { EMPTY } from "rxjs";
import { Flow, initialStateWalletSync, Step } from "~/renderer/reducers/walletSync";
import getWalletSyncEnvironmentParams from "@ledgerhq/live-common/walletSync/getEnvironmentParams";

export const INSTANCES: Array<TrustchainMember> = [
{
id: "currentInstance",
name: "macOS",
permissions: 112,
},
{
id: "2",
name: "Ipone 15",
permissions: 112,
},
];

export const lldWalletSyncFeatureFlag = {
lldWalletSync: {
enabled: true,
params: {
environment: "STAGING",
watchConfig: {
pollingInterval: 10000,
initialTimeout: 5000,
userIntentDebounce: 1000,
},
},
},
};

export const mockedSdk = getSdk(
true,
{
applicationId: 12,
name: "LLD Integration",
apiBaseUrl: getWalletSyncEnvironmentParams("STAGING").trustchainApiBaseUrl,
},
() => () => EMPTY,
);

export const walletSyncActivatedState = {
...initialStateWalletSync,
flow: Flow.WalletSyncActivated,
step: Step.WalletSyncActivated,
};

export const simpleTrustChain = {
rootId: "rootId",
deviceId: "deviceId",
trustchainId: "trustchainId",
};

export const WalletSyncTestApp = () => (
<>
<div id="modals"></div>
<WalletSyncRow />
</>
);
Original file line number Diff line number Diff line change
@@ -1,14 +1,6 @@
import React from "react";
import { render, screen, waitFor } from "tests/testUtils";
import WalletSyncRow from "~/renderer/screens/settings/sections/General/WalletSync";
import { simpleTrustChain, walletSyncActivatedState } from "../testHelper/helper";

const WalletSyncTestApp = () => (
<>
<div id="modals"></div>
<WalletSyncRow />
</>
);
import { WalletSyncTestApp, simpleTrustChain, walletSyncActivatedState } from "./shared";

jest.mock("../hooks/useQRCode", () => ({
useQRCode: () => ({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { renderHook } from "tests/testUtils";
import { INITIAL_STATE as INITIAL_STATE_SETTINGS } from "~/renderer/reducers/settings";
import { useWatchWalletSync } from "../hooks/useWatchWalletSync";
import {
INSTANCES,
lldWalletSyncFeatureFlag,
mockedSdk,
simpleTrustChain,
walletSyncActivatedState,
} from "./shared";

const INITIAL_STATE = {
walletSync: {
...walletSyncActivatedState,
instances: INSTANCES,
},
trustchain: {
trustchain: simpleTrustChain,
memberCredentials: {
pubkey: "currentInstance",
privatekey: "privatekey",
},
},
settings: {
...INITIAL_STATE_SETTINGS,
overriddenFeatureFlags: lldWalletSyncFeatureFlag,
},
};

jest.mock("../hooks/useTrustchainSdk", () => ({
useTrustchainSdk: () => ({
getMembers: (mockedSdk.getMembers = jest.fn()),
removeMember: (mockedSdk.removeMember = jest.fn()),
}),
}));

describe("useWatchWalletSync", () => {
it("should not run ledger sync watch loop when ff is disabled", async () => {
const { result, store } = renderHook(() => useWatchWalletSync(), {});

expect(store.getState().settings.overriddenFeatureFlags.lldWalletSync).not.toBeDefined();
expect(result.current.visualPending).toBe(false);
expect(result.current.walletSyncError).toBe(null);
expect(result.current.onUserRefresh).toBeInstanceOf(Function);
expect(result.current.onUserRefresh).not.toThrow();
});

it("should run ledger sync watch loop when ff is enabled", async () => {
const { result, store } = renderHook(() => useWatchWalletSync(), {
initialState: INITIAL_STATE,
});

expect(store.getState().settings.overriddenFeatureFlags.lldWalletSync.enabled).toBe(true);
expect(result.current.visualPending).toBe(true);
expect(result.current.walletSyncError).toBe(null);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,8 @@
*/
import React from "react";
import { render, screen, waitFor } from "tests/testUtils";
import WalletSyncRow from "~/renderer/screens/settings/sections/General/WalletSync";
import { initialStateWalletSync } from "~/renderer/reducers/walletSync";

const WalletSyncTestApp = () => (
<>
<div id="modals"></div>
<WalletSyncRow />
</>
);
import { WalletSyncTestApp } from "./shared";

describe("Rendering", () => {
it("should loads and displays WalletSync Row", async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useCallback, useEffect, useMemo, useState } from "react";
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
import { useDispatch, useSelector, useStore } from "react-redux";
import noop from "lodash/noop";
import { CloudSyncSDK } from "@ledgerhq/live-wallet/cloudsync/index";
Expand Down Expand Up @@ -120,16 +120,18 @@ export function useWatchWalletSync(): WalletSyncUserState {

const [visualPending, setVisualPending] = useState(true);
const [walletSyncError, setWalletSyncError] = useState<Error | null>(null);
const [onUserRefresh, setOnUserRefresh] = useState<() => void>(() => noop);
const onUserRefreshRef = useRef<() => void>(noop);
const state = useMemo(
() => ({ visualPending, walletSyncError, onUserRefresh }),
[visualPending, walletSyncError, onUserRefresh],
() => ({ visualPending, walletSyncError, onUserRefresh: onUserRefreshRef.current }),
[visualPending, walletSyncError],
);

// pull and push wallet sync loop
useEffect(() => {
if (!trustchain || !memberCredentials) {
setOnUserRefresh(() => noop);
const canNotRunWatchLoop = !featureWalletSync?.enabled || !trustchain || !memberCredentials;

if (canNotRunWatchLoop) {
onUserRefreshRef.current = noop;
setVisualPending(false);
setWalletSyncError(null);
return;
Expand Down Expand Up @@ -158,7 +160,7 @@ export function useWatchWalletSync(): WalletSyncUserState {
onTrustchainRefreshNeeded,
});

setOnUserRefresh(() => onUserRefreshIntent);
onUserRefreshRef.current = onUserRefreshIntent;

return unsubscribe;
}, [
Expand All @@ -169,7 +171,7 @@ export function useWatchWalletSync(): WalletSyncUserState {
memberCredentials,
onTrustchainRefreshNeeded,
saveUpdate,
featureWalletSync?.params?.watchConfig,
featureWalletSync,
]);

return state;
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ jest.mock("@ledgerhq/live-common/featureFlags/index", () => ({
useFeature: jest.fn(),
}));

jest.mock("../../../components/FirebaseFeatureFlags", () => ({
// eslint-disable-next-line @typescript-eslint/no-explicit-any
FirebaseFeatureFlagsProvider: ({ children }: { children: any }) => children,
}));

const mockedUseFeature = jest.mocked(useFeature);

const mockedOnChangeTransaction = jest.fn().mockImplementation(t => t);
Expand Down
19 changes: 19 additions & 0 deletions apps/ledger-live-desktop/tests/featureFlags.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { DEFAULT_FEATURES } from "@ledgerhq/live-common/featureFlags/index";

import { FeatureId, Feature, Features } from "@ledgerhq/types-live";

/* getFeature provides a basic behavior to mock how we retrieve feature flags values
* and allows overrides for our tests
*/
export const getFeature = <T extends FeatureId>({
key,
localOverrides,
}: {
key: T;
appLanguage?: string;
allowOverride?: boolean;
localOverrides?: { [key in FeatureId]?: Feature | undefined };
}): Features[T] => {
if (localOverrides?.[key]) return localOverrides?.[key] as Features[T];
return DEFAULT_FEATURES[key];
};
Loading

0 comments on commit d75616a

Please sign in to comment.