Skip to content

🧪 test: add isInRollout boundary tests #524

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sunnylqm wants to merge 2 commits into from
Closed

🧪 test: add isInRollout boundary tests #524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3682d2f
test: add isInRollout boundary tests
google-labs-jules[bot] Mar 7, 2026
d20de87
fix: resolve TypeScript errors for dynamic query imports and shadowed…
google-labs-jules[bot] Mar 7, 2026
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
4 changes: 4 additions & 0 deletions patch_client.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
sed -i '365s/info: CheckResult/updateInfo: CheckResult/' src/client.ts
sed -i '368s/info.updateUrl/updateInfo.updateUrl/' src/client.ts
sed -i '369s/info.hash/updateInfo.hash/' src/client.ts
sed -i '381s/info.hash/updateInfo.hash/' src/client.ts
Comment on lines +1 to +4
Copy link

@coderabbitai coderabbitai bot Mar 7, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu

echo "== addressed lines in src/client.ts =="
sed -n '365p;368p;369p;381p' src/client.ts

echo
echo "== old identifiers still present? =="
rg -n "info: CheckResult|info\\.updateUrl|info\\.hash" src/client.ts || true

Repository: reactnativecn/react-native-update

Length of output: 214

These line-numbered rewrites are already out of sync with src/client.ts.

On the checked-in file, Lines 368, 369, and 381 no longer contain the info.updateUrl / info.hash text this script expects, so it can exit successfully while applying nothing. Use content-based replacements and assert that each rewrite happened.

Suggested direction 
-sed -i '365s/info: CheckResult/updateInfo: CheckResult/' src/client.ts
-sed -i '368s/info.updateUrl/updateInfo.updateUrl/' src/client.ts
-sed -i '369s/info.hash/updateInfo.hash/' src/client.ts
-sed -i '381s/info.hash/updateInfo.hash/' src/client.ts
+sed -i 's/info: CheckResult/updateInfo: CheckResult/' src/client.ts
+sed -i 's/} = info;/} = updateInfo;/' src/client.ts
+sed -i 's/beforeDownloadUpdate(info)/beforeDownloadUpdate(updateInfo)/' src/client.ts
+sed -i 's/if (!info.update || !hash) {/if (!updateInfo.update || !hash) {/' src/client.ts
+grep -Fq 'updateInfo: CheckResult' src/client.ts
+grep -Fq '} = updateInfo;' src/client.ts
+grep -Fq 'beforeDownloadUpdate(updateInfo)' src/client.ts
+grep -Fq 'if (!updateInfo.update || !hash) {' src/client.ts
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
sed -i '365s/info: CheckResult/updateInfo: CheckResult/' src/client.ts
sed -i '368s/info.updateUrl/updateInfo.updateUrl/' src/client.ts
sed -i '369s/info.hash/updateInfo.hash/' src/client.ts
sed -i '381s/info.hash/updateInfo.hash/' src/client.ts
sed -i 's/info: CheckResult/updateInfo: CheckResult/' src/client.ts
sed -i 's/} = info;/} = updateInfo;/' src/client.ts
sed -i 's/beforeDownloadUpdate(info)/beforeDownloadUpdate(updateInfo)/' src/client.ts
sed -i 's/if (!info.update || !hash) {/if (!updateInfo.update || !hash) {/' src/client.ts
grep -Fq 'updateInfo: CheckResult' src/client.ts
grep -Fq '} = updateInfo;' src/client.ts
grep -Fq 'beforeDownloadUpdate(updateInfo)' src/client.ts
grep -Fq 'if (!updateInfo.update || !hash) {' src/client.ts
🧰 Tools
🪛 Shellcheck (0.11.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@patch_client.sh` around lines 1 - 4, The patch script uses fragile
line-numbered sed edits and fails silently when the expected snippets aren't
present; update patch_client.sh to perform content-based replacements for the
tokens "info: CheckResult" -> "updateInfo: CheckResult" and
"info.updateUrl"/"info.hash" -> "updateInfo.updateUrl"/"updateInfo.hash" by
matching and replacing those exact strings in src/client.ts (use
global/content-aware sed or perl regex) and then assert each replacement
succeeded (e.g., check the file diff, count replacements, or verify the new
strings exist) and exit non‑zero if any expected replacement did not occur so
the script cannot succeed without applying the intended edits.

3 changes: 3 additions & 0 deletions patch_client2.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
sed -i 's/} = info;/} = updateInfo;/' src/client.ts
sed -i 's/beforeDownloadUpdate(info)/beforeDownloadUpdate(updateInfo)/' src/client.ts
sed -i 's/if (!info.update || !hash) {/if (!updateInfo.update || !hash) {/' src/client.ts
3 changes: 3 additions & 0 deletions patch_tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
sed -i 's/await import('\''\.\.\/isInRollout?deterministic'\'')/await import('\''\.\.\/isInRollout?deterministic'\'' \/\* @vite-ignore \*\/) as any/' src/__tests__/isInRollout.test.ts
sed -i 's/await import('\''\.\.\/core?error'\'')/await import('\''\.\.\/core?error'\'' \/\* @vite-ignore \*\/) as any/' src/__tests__/core.test.ts
sed -i 's/await import('\''\.\.\/core?success'\'')/await import('\''\.\.\/core?success'\'' \/\* @vite-ignore \*\/) as any/' src/__tests__/core.test.ts
2 changes: 2 additions & 0 deletions src/__tests__/core.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ describe('core info parsing', () => {

// Use a unique query parameter to bypass cache if supported, or just rely on fresh environment per file.
// In Bun, you can sometimes use a cache buster if it's dynamic import.
// @ts-ignore
await import('../core?error');

expect(mockError).toHaveBeenCalledWith(
Expand Down Expand Up @@ -95,6 +96,7 @@ describe('core info parsing', () => {
emptyModule: {},
}));

// @ts-ignore
await import('../core?success');

expect(mockError).not.toHaveBeenCalled();
Expand Down
38 changes: 38 additions & 0 deletions src/__tests__/isInRollout.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { describe, expect, test, mock, beforeAll } from 'bun:test';

mock.module('../core', () => {
return {
cInfo: {
uuid: 'fixed-test-uuid',
},
};
});

describe('isInRollout', () => {
let isInRollout: (rollout: number) => boolean;

beforeAll(async () => {
// Dynamic import to ensure the mock is picked up
// @ts-ignore
const module = await import('../isInRollout?deterministic');
isInRollout = module.isInRollout;
});

test('returns false when rollout is 0', () => {
expect(isInRollout(0)).toBe(false);
});

test('returns false when rollout is less than or equal to the hash modulo (79)', () => {
// 79 < 79 is false
expect(isInRollout(79)).toBe(false);
});

test('returns true when rollout is strictly greater than the hash modulo (80)', () => {
// 79 < 80 is true
expect(isInRollout(80)).toBe(true);
});

test('returns true when rollout is 100', () => {
expect(isInRollout(100)).toBe(true);
});
});
10 changes: 10 additions & 0 deletions src/__tests__/setup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,13 @@ mock.module('../i18n', () => {
},
};
});

mock.module('react-native/Libraries/Core/ReactNativeVersion', () => {
return {
version: {
major: 0,
minor: 70,
patch: 0,
},
};
});
8 changes: 4 additions & 4 deletions src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ export class Pushy {
return server.backups;
};
downloadUpdate = async (
info: CheckResult,
updateInfo: CheckResult,
onDownloadProgress?: (data: ProgressData) => void,
) => {
const {
Expand All @@ -374,15 +374,15 @@ export class Pushy {
name,
description = '',
metaInfo,
} = info;
} = updateInfo;
if (
this.options.beforeDownloadUpdate &&
(await this.options.beforeDownloadUpdate(info)) === false
(await this.options.beforeDownloadUpdate(updateInfo)) === false
) {
log('beforeDownloadUpdate returned false, skipping download');
return;
}
if (!info.update || !hash) {
if (!updateInfo.update || !hash) {
return;
}
if (rolledBackVersion === hash) {
Expand Down