Skip to content
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.

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

[NoQA] e2e: handle process id has changed #46279

Merged
merged 2 commits into from
Jul 30, 2024
Merged
Changes from 1 commit
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
Next Next commit
e2e: handle process id has changed
  • Loading branch information
kirillzyusko committed Jul 26, 2024

Verified

This commit was signed with the committer’s verified signature.
kidroca Peter Velkov
commit 4d2ca3b8af35543aa53987253152c888b93c34ec
45 changes: 45 additions & 0 deletions patches/@perf-profiler+android+0.13.0+001+pid-changed.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
diff --git a/node_modules/@perf-profiler/android/dist/src/commands/platforms/UnixProfiler.js b/node_modules/@perf-profiler/android/dist/src/commands/platforms/UnixProfiler.js
index 657c3b0..c97e363 100644
--- a/node_modules/@perf-profiler/android/dist/src/commands/platforms/UnixProfiler.js
+++ b/node_modules/@perf-profiler/android/dist/src/commands/platforms/UnixProfiler.js
@@ -113,7 +113,7 @@ class UnixProfiler {
}
pollPerformanceMeasures(bundleId, { onMeasure, onStartMeasuring = () => {
// noop by default
- }, }) {
+ }, onPidChanged = () => {}}) {
let initialTime = null;
let previousTime = null;
let cpuMeasuresAggregator = new CpuMeasureAggregator_1.CpuMeasureAggregator(this.getCpuClockTick());
@@ -170,6 +170,7 @@ class UnixProfiler {
previousTime = timestamp;
}, () => {
logger_1.Logger.warn("Process id has changed, ignoring measures until now");
+ onPidChanged();
reset();
});
}
diff --git a/node_modules/@perf-profiler/android/src/commands/platforms/UnixProfiler.ts b/node_modules/@perf-profiler/android/src/commands/platforms/UnixProfiler.ts
index be26fe6..0473f78 100644
--- a/node_modules/@perf-profiler/android/src/commands/platforms/UnixProfiler.ts
+++ b/node_modules/@perf-profiler/android/src/commands/platforms/UnixProfiler.ts
@@ -105,9 +105,11 @@ export abstract class UnixProfiler implements Profiler {
onStartMeasuring = () => {
// noop by default
},
+ onPidChanged = () => {},
Copy link
Contributor

Choose a reason for hiding this comment

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

do we have a upstream PR for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hannojg not yet. I'd like to test how stable this fix will be in Expensify codebase and if it's pretty stable, then I'll create upstream PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi! I'm going through our patches and would like to know if we have an upstream PR for this now :)

}: {
onMeasure: (measure: Measure) => void;
onStartMeasuring?: () => void;
+ onPidChanged?: () => void;
}
) {
let initialTime: number | null = null;
@@ -187,6 +189,7 @@ export abstract class UnixProfiler implements Profiler {
},
() => {
Logger.warn("Process id has changed, ignoring measures until now");
+ onPidChanged();
reset();
}
);
12 changes: 12 additions & 0 deletions patches/@perf-profiler+types+0.8.0+001+pid-changed.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
diff --git a/node_modules/@perf-profiler/types/dist/index.d.ts b/node_modules/@perf-profiler/types/dist/index.d.ts
index 0d0f55f..ef7f864 100644
--- a/node_modules/@perf-profiler/types/dist/index.d.ts
+++ b/node_modules/@perf-profiler/types/dist/index.d.ts
@@ -80,6 +80,7 @@ export interface ScreenRecorder {
export interface ProfilerPollingOptions {
onMeasure: (measure: Measure) => void;
onStartMeasuring?: () => void;
+ onPidChanged?: () => void;
}
export interface Profiler {
pollPerformanceMeasures: (bundleId: string, options: ProfilerPollingOptions) => {
13 changes: 10 additions & 3 deletions tests/e2e/server/index.ts
Original file line number Diff line number Diff line change
@@ -22,12 +22,15 @@ type TestResultListener = (testResult: TestResult) => void;

type AddListener<TListener> = (listener: TListener) => () => void;

type ClearAllListeners = () => void;

type ServerInstance = {
setTestConfig: (testConfig: TestConfig) => void;
getTestConfig: () => TestConfig;
addTestStartedListener: AddListener<TestStartedListener>;
addTestResultListener: AddListener<TestResultListener>;
addTestDoneListener: AddListener<TestDoneListener>;
clearAllTestDoneListeners: ClearAllListeners;
forceTestCompletion: () => void;
setReadyToAcceptTestResults: (isReady: boolean) => void;
isReadyToAcceptTestResults: boolean;
@@ -70,7 +73,7 @@ const getPostJSONRequestData = <TRequestData extends RequestData>(req: IncomingM
});
};

const createListenerState = <TListener>(): [TListener[], AddListener<TListener>] => {
const createListenerState = <TListener>(): [TListener[], AddListener<TListener>, ClearAllListeners] => {
const listeners: TListener[] = [];
const addListener = (listener: TListener) => {
listeners.push(listener);
@@ -81,8 +84,11 @@ const createListenerState = <TListener>(): [TListener[], AddListener<TListener>]
}
};
};
const clearAllListeners = () => {
listeners.splice(0, listeners.length);
};

return [listeners, addListener];
return [listeners, addListener, clearAllListeners];
};

/**
@@ -98,7 +104,7 @@ const createListenerState = <TListener>(): [TListener[], AddListener<TListener>]
const createServerInstance = (): ServerInstance => {
const [testStartedListeners, addTestStartedListener] = createListenerState<TestStartedListener>();
const [testResultListeners, addTestResultListener] = createListenerState<TestResultListener>();
const [testDoneListeners, addTestDoneListener] = createListenerState<TestDoneListener>();
const [testDoneListeners, addTestDoneListener, clearAllTestDoneListeners] = createListenerState<TestDoneListener>();
let isReadyToAcceptTestResults = true;

const setReadyToAcceptTestResults = (isReady: boolean) => {
@@ -229,6 +235,7 @@ const createServerInstance = (): ServerInstance => {
addTestStartedListener,
addTestResultListener,
addTestDoneListener,
clearAllTestDoneListeners,
forceTestCompletion,
start: () =>
new Promise<void>((resolve) => {
20 changes: 17 additions & 3 deletions tests/e2e/testRunner.ts
Original file line number Diff line number Diff line change
@@ -52,7 +52,7 @@ const setConfigPath = (configPathParam: string | undefined) => {
if (!configPath?.startsWith('.')) {
configPath = `./${configPath}`;
}
const customConfig = require<CustomConfig>(configPath).default;
const customConfig = (require(configPath) as CustomConfig).default;
config = Object.assign(defaultConfig, customConfig);
};

@@ -150,8 +150,7 @@ const runTests = async (): Promise<void> => {
Logger.log('Launching', appPackage);
await launchApp('android', appPackage, config.ACTIVITY_PATH, launchArgs);

MeasureUtils.start(appPackage);
await withFailTimeout(
const {promise, resetTimeout} = withFailTimeout(
new Promise<void>((resolve) => {
const removeListener = server.addTestDoneListener(() => {
Logger.success(iterationText);
@@ -194,9 +193,22 @@ const runTests = async (): Promise<void> => {
removeListener();
resolve();
});
MeasureUtils.start(appPackage, {
onAttachFailed: async () => {
MeasureUtils.stop();
resetTimeout();
removeListener();
// something went wrong, let's wait a little bit and try again
await sleep(5000);
// simply restart the test
await runTestIteration(appPackage, iterationText, branch, launchArgs);
resolve();
},
});
}),
iterationText,
);
await promise;

Logger.log('Killing', appPackage);
await killApp('android', appPackage);
@@ -256,6 +268,8 @@ const runTests = async (): Promise<void> => {
// We run each test multiple time to average out the results
for (let testIteration = 0; testIteration < config.RUNS; testIteration++) {
const onError = (e: Error) => {
MeasureUtils.stop();
server.clearAllTestDoneListeners();
errorCountRef.errorCount += 1;
if (testIteration === 0 || errorCountRef.errorCount === errorCountRef.allowedExceptions) {
Logger.error("There was an error running the test and we've reached the maximum number of allowed exceptions. Stopping the test run.");
5 changes: 4 additions & 1 deletion tests/e2e/utils/measure.ts
Original file line number Diff line number Diff line change
@@ -11,14 +11,17 @@ const POLLING_STOPPED = {
};
let polling = POLLING_STOPPED;

const start = (bundleId: string) => {
const start = (bundleId: string, {onAttachFailed}: {onAttachFailed: () => Promise<void>}) => {
// clear our measurements results
measures = [];

polling = profiler.pollPerformanceMeasures(bundleId, {
onMeasure: (measure: Measure) => {
measures.push(measure);
},
onPidChanged: () => {
onAttachFailed();
},
});
};

15 changes: 11 additions & 4 deletions tests/e2e/utils/withFailTimeout.ts
Original file line number Diff line number Diff line change
@@ -3,9 +3,13 @@ import CONFIG from '../config';
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing -- nullish coalescing doesn't achieve the same result in this case
const TIMEOUT = Number(process.env.INTERACTION_TIMEOUT || CONFIG.INTERACTION_TIMEOUT);

const withFailTimeout = (promise: Promise<void>, name: string): Promise<void> =>
new Promise((resolve, reject) => {
const timeoutId = setTimeout(() => {
const withFailTimeout = (promise: Promise<void>, name: string): {promise: Promise<unknown>; resetTimeout: () => void} => {
let timeoutId: NodeJS.Timeout;
const resetTimeout = () => {
clearTimeout(timeoutId);
};
const race = new Promise((resolve, reject) => {
timeoutId = setTimeout(() => {
reject(new Error(`"${name}": Interaction timed out after ${(TIMEOUT / 1000).toFixed(0)}s`));
}, TIMEOUT);

@@ -17,8 +21,11 @@ const withFailTimeout = (promise: Promise<void>, name: string): Promise<void> =>
reject(e);
})
.finally(() => {
clearTimeout(timeoutId);
resetTimeout();
});
});

return {promise: race, resetTimeout};
};

export default withFailTimeout;
Loading