arquillian/appclient wip progress logging changes #1647
Conversation
Signed-off-by: Scott Marlow <[email protected]>
|
@jamezp please review when you have a chance. |
| boolean sawStatus = false; | ||
| MainStatus status = MainStatus.NOT_RUN; | ||
| String reason = "None"; | ||
| String description = "None"; | ||
| for (String line : lines) { | ||
| System.out.println(line); | ||
| log.finer(line); |
There was a problem hiding this comment.
This part could end up making it look a little odd if it's already a formatted log line. I also feel this should be at level fine not finer.
| @@ -117,6 +119,7 @@ public TestResult invoke(TestMethodExecutor testMethodExecutor) { | |||
| break; | |||
| } | |||
| result.addDescription(description); | |||
| log.info(result.toString() + ": " + description + ": " + vehicleArchiveName + ": " + appArchiveName); | |||
There was a problem hiding this comment.
I think this might look a little better with something like:
log.log(level, String.format("%s_%s [%s] %s : %s", vehicaleArchiveName, appArchiveName, result.getStatus(), description, result);
Note I'm not sure how the vehicaleArchiveName and appArchiveName should be formatted.
The level should be set in the in the switch statement right above this log line IMO. Then for ERROR and FAILED status we could set the level to Level.SEVERE.
There was a problem hiding this comment.
Thanks, switching over to this style. I don't think the appclient jar or vehicle names are what I really wanted to show here. Showing the EAR name would be nicer (based on how we current include more information in the ear name). Revising...
There was a problem hiding this comment.
Will update this to show the new output with:
result.addDescription(description);
log.log(Level.INFO, String.format("[%s] %s : %s", result.getStatus(), description, result));
I have to verify as currently, I have the platformtck/glassfish-runner/persistence-platform-tck/jakartaeetck/bin/ts.jte |
…tput Signed-off-by: Scott Marlow <[email protected]>
| @@ -117,6 +120,7 @@ public TestResult invoke(TestMethodExecutor testMethodExecutor) { | |||
| break; | |||
| } | |||
| result.addDescription(description); | |||
| log.log(Level.FINE, String.format("[%s] %s : %s", result.getStatus(), description, result)); | |||
There was a problem hiding this comment.
I still feel like it makes sense to make the level configurable. Meaning, use a level based on the status.
Make the appclient logging a little less verbose by default at line 86.
I'm not yet sure why the logged information at 122 doesn't show the ear deployment file name (instead it seems to show the client jar (which is part of the ear). It would be nice to show the top level deployment as well.