Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 120 additions & 0 deletions src/frontend/screens/Exchange/ExchangeScreenContent.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import * as React from 'react';
import type {SyncState} from '@comapeo/core-react';
import {NavigationContainer} from '@react-navigation/native';
import {createNativeStackNavigator} from '@react-navigation/native-stack';
import {QueryClient, QueryClientProvider} from '@tanstack/react-query';
import {render, screen} from '@testing-library/react-native';
import {IntlProvider} from 'react-intl';

import {ExchangeScreenContent} from './ExchangeScreenContent';

let mockDataSyncProgress: number | null = null;

const mockSync = {
connectServers: jest.fn(),
disconnectServers: jest.fn(),
setAutostopDataSyncTimeout: jest.fn(),
stop: jest.fn(),
};

jest.mock('@comapeo/core-react', () => ({
...jest.requireActual('@comapeo/core-react'),
useDataSyncProgress: () => mockDataSyncProgress,
useIsArchiveDevice: () => ({data: false}),
useStartSync: () => ({mutate: jest.fn()}),
}));

jest.mock('../../contexts/ActiveProjectContext', () => ({
useActiveProject: () => ({
projectId: 'project-id',
projectApi: {$sync: mockSync},
}),
}));

jest.mock('../../hooks/server/projects', () => ({
useActiveArchiveServer: () => undefined,
}));

jest.mock('../../hooks/useLocalDiscoveryState', () => ({
useLocalDiscoveryState: () => 'CoMapeo Test Wi-Fi',
}));

const serverPeerSyncState = {
initial: {isSyncEnabled: true, want: 0, wanted: 0},
data: {isSyncEnabled: true, want: 5, wanted: 0},
};

const syncingWithServer = {
initial: {isSyncEnabled: true},
data: {isSyncEnabled: true},
remoteDeviceSyncState: {server: serverPeerSyncState},
} satisfies SyncState;

const Stack = createNativeStackNavigator<{Sync: undefined}>();

const queryClient = new QueryClient();

const Wrapper = ({children}: {children: React.ReactNode}) => (
<QueryClientProvider client={queryClient}>
<IntlProvider locale="en">{children}</IntlProvider>
</QueryClientProvider>
);

async function renderExchangeScreen(syncState: SyncState) {
const {unmount} = await render(
<React.Suspense fallback={null}>
<NavigationContainer>
<Stack.Navigator>
<Stack.Screen name="Sync">
{() => <ExchangeScreenContent syncState={syncState} />}
</Stack.Screen>
</Stack.Navigator>
</NavigationContainer>
</React.Suspense>,
{wrapper: Wrapper},
);
return unmount;
}

describe('ExchangeScreenContent server connection on leaving the screen', () => {
beforeEach(() => {
jest.clearAllMocks();
});

test('keeps the server connected while data sync is still in progress', async () => {
mockDataSyncProgress = 0.4;
const unmount = await renderExchangeScreen(syncingWithServer);

await expect(screen.findByText('Exchanging...')).resolves.toBeVisible();
expect(mockSync.connectServers).toHaveBeenCalled();

await unmount();

expect(mockSync.disconnectServers).not.toHaveBeenCalled();
});

test('disconnects the server once data sync is complete', async () => {
mockDataSyncProgress = 1;
const unmount = await renderExchangeScreen(syncingWithServer);

await expect(screen.findByText('Complete!')).resolves.toBeVisible();

await unmount();

expect(mockSync.disconnectServers).toHaveBeenCalledTimes(1);
});

test('disconnects the server when data sync is not enabled', async () => {
mockDataSyncProgress = null;
const unmount = await renderExchangeScreen({
...syncingWithServer,
data: {isSyncEnabled: false},
});

await expect(screen.findByText('Devices found.')).resolves.toBeVisible();

await unmount();

expect(mockSync.disconnectServers).toHaveBeenCalledTimes(1);
});
});
17 changes: 16 additions & 1 deletion src/frontend/screens/Exchange/ExchangeScreenContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -154,14 +154,29 @@ export const ExchangeScreenContent = ({syncState}: {syncState: SyncState}) => {
dataSyncEnabled: syncState.data.isSyncEnabled,
});

// Cutting the server websocket mid-sync leaves it with partial data.
const shouldDisconnectServersWhenLeavingScreen =

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this variable, or could we just use shouldDisconnectServersRef.current, and avoid the useEffect to update it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Half of it. The variable is gone (afac571), but the effect has to stay: writing ref.current during render trips the react-hooks/refs rule in this repo's eslint config ("Cannot update ref during render"), so the only lint-clean way to keep the latest value readable from the focus cleanup is an effect. It now runs without a deps array and assigns directly, which is as small as I could make it.

!syncState.data.isSyncEnabled || syncStage.name === 'complete-full';

// Ref, not a dependency: re-running the focus effect would reconnect servers.
const shouldDisconnectServersRef = React.useRef(
shouldDisconnectServersWhenLeavingScreen,
);
React.useEffect(() => {
shouldDisconnectServersRef.current =
shouldDisconnectServersWhenLeavingScreen;
}, [shouldDisconnectServersWhenLeavingScreen]);

useFocusEffect(
React.useCallback(() => {
// Unset sync autostop when screen mounts
projectApi.$sync.setAutostopDataSyncTimeout(null);
// Connects to Servers
projectApi.$sync.connectServers();
return () => {
projectApi.$sync.disconnectServers();
if (shouldDisconnectServersRef.current) {
projectApi.$sync.disconnectServers();
}
};
}, [projectApi]),
);
Expand Down
Loading