Conversation
Signed-off-by: slievrly <slievrly@163.com>
There was a problem hiding this comment.
Pull request overview
Prevents a null-pointer failure in the e2e test runner when the configured e2e directory cannot be listed (e.g., missing, not a directory, or inaccessible).
Changes:
- Add a null-check for
File.listFiles()results inSkyWalkingController.runE2ETests(). - Log an error and exit early when directory listing fails.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| LOGGER.error("Failed to list files in directory: " + this.e2eDir); | ||
| LOGGER.error("Please check if the directory exists and is accessible."); |
There was a problem hiding this comment.
These new log lines use string concatenation; elsewhere in this file SLF4J parameterized logging is used (e.g., LOGGER.error("Test Directory: {}", ...)). Prefer {} placeholders here as well to avoid unnecessary string building and keep logging consistent.
| File e2eDir = new File(this.e2eDir); | ||
| List<String> passedProjects = new ArrayList<>(); | ||
| File[] files = e2eDir.listFiles(); |
There was a problem hiding this comment.
The local variable File e2eDir = new File(this.e2eDir); shadows the e2eDir field name; with the added error handling it’s now easy to confuse this.e2eDir (String) vs e2eDir (File). Renaming the local variable (e.g., e2eDirFile) would make this block clearer.
| File e2eDir = new File(this.e2eDir); | |
| List<String> passedProjects = new ArrayList<>(); | |
| File[] files = e2eDir.listFiles(); | |
| File e2eDirFile = new File(this.e2eDir); | |
| List<String> passedProjects = new ArrayList<>(); | |
| File[] files = e2eDirFile.listFiles(); |
| File[] files = e2eDir.listFiles(); | ||
| if (files == null) { | ||
| LOGGER.error("Failed to list files in directory: " + this.e2eDir); | ||
| LOGGER.error("Please check if the directory exists and is accessible."); |
There was a problem hiding this comment.
File.listFiles() returns null for multiple reasons (path doesn’t exist, not a directory, I/O/permission issues). Consider checking e2eDir.exists() / e2eDir.isDirectory() and logging the absolute path + those flags so failures are easier to diagnose.
1 participant
Ⅰ. Describe what this PR did
Ⅱ. Does this pull request fix one issue?
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews