-
-
Notifications
You must be signed in to change notification settings - Fork 7
feat(dvt): add ProcessControl instrument for process management #136
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
base: main
Are you sure you want to change the base?
Conversation
| */ | ||
| export interface ProcessControlService { | ||
| /** | ||
| * Send a signal to a process |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens if pid does not exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same below
| kill(pid: number): Promise<void>; | ||
|
|
||
| /** | ||
| * Disable memory limits for a specific process |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how does this work? what are the default memory limits?
| * @param bundleId The bundle identifier | ||
| * @returns The process identifier (PID) | ||
| */ | ||
| processIdentifierForBundleIdentifier(bundleId: string): Promise<number>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens if bundleId does not exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider including a verb to the method name
| * Disable memory limits for a specific process | ||
| * @param pid The process identifier | ||
| */ | ||
| disableMemoryLimitForPid(pid: number): Promise<void>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens if this method is called twice on the same process?
| const result = await this.channel!.receivePlist(); | ||
|
|
||
| if (typeof result !== 'number') { | ||
| throw new Error(`Failed to launch process: ${JSON.stringify(result)}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please document this behavior
| * Encodes JavaScript objects into NSKeyedArchiver format | ||
| * capable of satisfying NSSecureCoding requirements. | ||
| */ | ||
| export class NSKeyedArchiverEncoder { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this class covered by unit tests?
|
|
||
| const log = logger.getLogger('ProcessControl.test'); | ||
|
|
||
| describe('ProcessControl Service', function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to have some tests for outputEvents generator, we can add steps/requirements as comments in tests if required.
No description provided.