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

Limiting ProcessTree.killAll to InboundAgentRule.after #802

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

jglick
Copy link
Member

@jglick jglick commented Jul 10, 2024

#795 (comment) I neglected to consider direct call to InboundAgentRule.stop from test code, which should not kill subprocesses, only the agent JVM itself. Tested in workflow-durable-task-step on all test cases in AgentErrorConditionTest & ExecutorStepTest; also verified that the proprietary test suite originally motivating #795 continues to pass with this, and that subprocesses are still terminated at the end of the test.

private final ConcurrentMap<String, Process> procs = new ConcurrentHashMap<>();
private final Map<String, Process> procs = Collections.synchronizedMap(new HashMap<>());
Copy link
Member Author

Choose a reason for hiding this comment

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

(to allow null values)

@jglick jglick merged commit 7742810 into jenkinsci:master Jul 10, 2024
14 checks passed
@jglick jglick deleted the ProcessTree.killAll branch July 10, 2024 16:19
basil added a commit to basil/plugin-compat-tester that referenced this pull request Jul 10, 2024
basil added a commit to jenkinsci/plugin-compat-tester that referenced this pull request Jul 10, 2024
@basil
Copy link
Member

basil commented Jul 10, 2024

Thank you very much! Confirming that my Jetty 12 test matrix is back to green after this PR.

basil added a commit to basil/jenkins-test-harness that referenced this pull request Jul 15, 2024
basil added a commit that referenced this pull request Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants