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
Show file tree
Hide file tree
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
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

}: {
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
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -81,8 +84,11 @@ const createListenerState = <TListener>(): [TListener[], AddListener<TListener>]
}
};
};
const clearAllListeners = () => {
listeners.splice(0, listeners.length);
};

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

/**
Expand All @@ -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) => {
Expand Down Expand Up @@ -229,6 +235,7 @@ const createServerInstance = (): ServerInstance => {
addTestStartedListener,
addTestResultListener,
addTestDoneListener,
clearAllTestDoneListeners,
forceTestCompletion,
start: () =>
new Promise<void>((resolve) => {
Expand Down
20 changes: 17 additions & 3 deletions tests/e2e/testRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.");
Expand Down
5 changes: 4 additions & 1 deletion tests/e2e/utils/measure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@ const POLLING_STOPPED = {
};
let polling = POLLING_STOPPED;

const start = (bundleId: string) => {
const start = (bundleId: string, {onAttachFailed}: {onAttachFailed: () => Promise<void>}) => {
kirillzyusko marked this conversation as resolved.
Show resolved Hide resolved
// clear our measurements results
measures = [];

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

Expand Down
15 changes: 11 additions & 4 deletions tests/e2e/utils/withFailTimeout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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