Enh/ap 25396 system info collector to collect more than 5 processes #14
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the system diagnostics functionality by increasing the number of processes collected and improving JSON serialization of diagnostic instructions.
- Increased top processes collection limit from 5 to 100
- Added custom Path serializer to produce plain path strings instead of file:// URLs
- Enhanced diagnostics directory README with JSON configuration examples
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| SystemInfoCollector.java | Increased process collection limit from 5 to 100 with explanatory comment |
| DiagnosticInstructions.java | Added PathAsStringSerializer to serialize Path objects as plain strings |
| ApplicationHealthEarlyStartup.java | Enhanced diagnostics README file with JSON configuration example |
| DiagnosticInstructionsSerializationTest.java | Added comprehensive serialization/deserialization tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assertTrue("System info should be false", !instructions.systemInfo()); | ||
| assertTrue("JVM info should be false", !instructions.jvmInfo()); | ||
| assertTrue("GC info should be false", !instructions.gcInfo()); | ||
| assertTrue("Application health should be false", !instructions.applicationHealth()); | ||
| assertTrue("KNIME info should be false", !instructions.knimeInfo()); | ||
| assertTrue("Workflow managers should be false", !instructions.workflowManagers()); | ||
| assertTrue("Thread dump should be false", !instructions.threadDump()); |
There was a problem hiding this comment.
Use assertFalse instead of assertTrue with negation (!). This makes the test assertions clearer and produces more helpful failure messages. For example, replace assertTrue(\"System info should be false\", !instructions.systemInfo()) with assertFalse(\"System info should be false\", instructions.systemInfo()).
There was a problem hiding this comment.
@copilot add a new commit to this branch implementing this feedback
|
@bernd-wiswedel I've opened a new pull request, #15, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
knime-github
deleted the
enh/AP-25396-system-info-collector-to-collect-more-than-5-processes
branch
|
The topic branch (also for the adjacent ticket #15) got auto-magically deleted without merge. This needs investigation, probably was done by a left-over bitbucket->github jenkins job (KNIME switched to using GH as primary git repo only yesterday). This ticket will be discussed via separate PR. |
No description provided.