Add ability storing of webdriver console and network logs after failing tests #8926
Conversation
|
ci-build |
benoitf
added
status/code-review
| @@ -93,6 +94,10 @@ | |||
| @Named("tests.htmldumps_dir") | |||
| private String htmldumpsDir; | |||
|
|
|||
| @Inject | |||
| @Named("tests.webDriverLogsDir") | |||
There was a problem hiding this comment.
use the same test properties naming pattern, please: tests.webDriverLogsDir > tests.webdriverlogs_dir
| try { | ||
| String testReference = getTestReference(result); | ||
| String filename = NameGenerator.generate(testReference + "_", 4) + ".log"; | ||
| Path webdriverLogDirectory = Paths.get(webDriverLogsDir, filename); |
There was a problem hiding this comment.
webdriverLogDirectory > webdriverLogsDirectory
|
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/4483/ |
| webDrivers.forEach(webDriver -> getWebDriverLog(result, webDriver)); | ||
| } | ||
|
|
||
| private void getWebDriverLog(ITestResult result, SeleniumWebDriver webDriver) { |
There was a problem hiding this comment.
get usually means returning something.
This method returns nothing. It stores logs of webdriver into the filesystem. So, method name should be rephrased appropriately, for example: storeWebDriverLogs
| @@ -25,27 +27,115 @@ | |||
| * Read and store browser logs to the test logs. Log level and type are defined in {@link | |||
| * org.eclipse.che.selenium.core.SeleniumWebDriver#doCreateDriver(URL)} | |||
| */ | |||
| @Singleton | |||
| public class BrowserLogsUtil { | |||
There was a problem hiding this comment.
Class name is outdated so as it is not about just BROWSER logs, but PERFORMANCE logs as well. So, lets make the name more general: WebDriverLogsReader
| @@ -25,27 +27,115 @@ | |||
| * Read and store browser logs to the test logs. Log level and type are defined in {@link | |||
There was a problem hiding this comment.
Javadocs doesn't take into account performance logs. And this class stores nothing.
|
|
||
| /** | ||
| * read logs from browser console | ||
| * | ||
| * @return log messages from browser console | ||
| */ | ||
| public List<LogEntry> getLogs() { | ||
| public static List<LogEntry> getConsoleLogs(WebDriver seleniumWebDriver) { |
There was a problem hiding this comment.
lets name the method as readBrowserLogs
| * | ||
| * @return all types of performance logs | ||
| */ | ||
| public static List<LogEntry> getPerformanceLogs(WebDriver seleniumWebDriver) { |
There was a problem hiding this comment.
lets name the method as readPerformanceLogs
| /** store browser logs to the test logs */ | ||
| public void storeLogs() { | ||
| getLogs().forEach(logEntry -> LOG.info("{} {}", logEntry.getLevel(), logEntry.getMessage())); | ||
| public static void storeLogsToConsoleOutput(WebDriver seleniumWebDriver) { |
There was a problem hiding this comment.
storeLogsToConsoleOutput > logBrowserLogs
| } | ||
|
|
||
| /** filter data and get requests/responses that has been sent on CHE /api/ URL */ | ||
| public static String getNetworkDataSentOnCheApi(WebDriver seleniumWebDriver) { |
There was a problem hiding this comment.
getNetworkDataSentOnCheApi > readCheNetworkTraffic
| JsonParser jsonParser = new JsonParser(); | ||
| getPerformanceLogs(seleniumWebDriver) | ||
| .forEach( | ||
| logEntry -> { |
There was a problem hiding this comment.
I would suggest to use stream.filter() method explicitly to simplify code, of replace lambda with for-each, if statements.
| * @param requestMessage json representation of the message object from the log | ||
| * @return info about request from the WebDriver | ||
| */ | ||
| private static String getRequestsSentOnChe(JsonObject requestMessage) { |
There was a problem hiding this comment.
getRequestsSentOnChe > extractCheRequests
| * @param requestMessage json representation of the message object from the log | ||
| * @return info about request from the WebDriver | ||
| */ | ||
| private static String getResponsesSentOnChe(JsonObject requestMessage) { |
There was a problem hiding this comment.
getResponsesSentOnChe > extractCheResponces
There was a problem hiding this comment.
I am sorry: typo from my side: extractCheResponces > extractCheResponses
| return responceInfo.toString(); | ||
| } | ||
|
|
||
| private static boolean isLogEntryContainsApiUrl(JsonObject node) { |
There was a problem hiding this comment.
isLogEntryContainsApiUrl > isNodeFromCheTraffic
| } | ||
|
|
||
| private static boolean isLogEntryContainsApiUrl(JsonObject node) { | ||
| return (node.get("url").isJsonNull()) ? false : node.get("url").getAsString().contains("/api/"); |
There was a problem hiding this comment.
there could be requests to other resources with "/api/" in URL. It's more reliable to check on Che hostname as well.
| * | ||
| * @return logs from browser console and requests/responses on CHE api | ||
| */ | ||
| public static String getCombinedLogs(WebDriver seleniumWebDriver) { |
There was a problem hiding this comment.
getCombinedLogs > combineBrowserAndCheTrafficLogs
There was a problem hiding this comment.
This method return NetworkTraffic + WebDriver java script console logs
There was a problem hiding this comment.
I understood it. But method name getCombinedLogs says nothing about content of getting logs.
There was a problem hiding this comment.
Or name it getAllLogs, and fix comment:
combine the Network and Browser logs > get all available logs of web driver.
| @@ -48,6 +50,7 @@ | |||
| @Inject private CodenvyEditor editor; | |||
| @Inject private TestProjectServiceClient testProjectServiceClient; | |||
| @Inject private BrowserLogsUtil browserLogsUtil; | |||
| @Inject private SeleniumWebDriver webDriver; | |||
There was a problem hiding this comment.
it's better to avoid injection of WebDriver into the test. Lets inject webdriver in the same way as it was done in SeleniumWebDriverHelper class.
| @@ -47,7 +48,7 @@ | |||
| @Inject private ProjectExplorer projectExplorer; | |||
| @Inject private CodenvyEditor editor; | |||
| @Inject private TestProjectServiceClient testProjectServiceClient; | |||
| @Inject private BrowserLogsUtil browserLogsUtil; | |||
| @Inject private SeleniumWebDriver webDriver; | |||
There was a problem hiding this comment.
it's better to avoid injection of WebDriver into the test. Lets inject webdriver in the same way as it was done in SeleniumWebDriverHelper class.
|
ci-test |
|
ci-test build report: |
Signed-off-by: Vitalii Parfonov <vparfonov@redhat.com>
…int of browser logs Signed-off-by: Dmytro Nochevnov <dnochevnov@codenvy.com>
Signed-off-by: Dmytro Nochevnov <dnochevnov@codenvy.com>
Signed-off-by: Dmytro Nochevnov <dnochevnov@codenvy.com>
Signed-off-by: Dmytro Nochevnov <dnochevnov@codenvy.com>
Signed-off-by: Dmytro Nochevnov <dnochevnov@codenvy.com>
An example of web driver logsBrowser console logs: --------------------- 2018-03-03 04:34:00,038 WARNING http://172.19.20.32:5050/auth/realms/che/protocol/openid-connect/auth?client_id=che-public&redirect_uri=http%3A%2F%2F172.19.20.32%3A8080%2Fuser1520087579%2Fworkspace0etcnt&state=268d3025-2488-41aa-90a7-46d52078fc9b&nonce=ee508571-270e-4326-95d7-969a06b8ef07&response_mode=fragment&response_type=code&scope=openid - This page includes a password or credit card input in a non-secure context. A warning will be added to the URL bar in Chrome 56 (Jan 2017). For more information, see https://goo.gl/zmWq3m. |
| "createMoveRefactoring started in " | ||
| + System.currentTimeMillis() | ||
| + " ms with " | ||
| + cmr.toString()); |
There was a problem hiding this comment.
Use please LOG.info("createMoveRefactoring started in {} " ms with " {}", System.currentTimeMillis(), cmr.toString())
| "createMoveRefactoring fail in " | ||
| + System.currentTimeMillis() | ||
| + " ms with exception " | ||
| + e.getMessage()); |
There was a problem hiding this comment.
| + System.currentTimeMillis() | ||
| + " ms with session id " | ||
| + moveRefactoringSession); | ||
| return moveRefactoringSession; |
There was a problem hiding this comment.
| LOG.error( | ||
| "createMoveRefactoring finished in " | ||
| + System.currentTimeMillis() | ||
| + " ms with with RefactoringException"); |
There was a problem hiding this comment.
| "createRenameRefactoring started in " | ||
| + System.currentTimeMillis() | ||
| + " ms with " | ||
| + settings.toString()); |
There was a problem hiding this comment.
| LOG.error( | ||
| "createRenameRefactoring fail in " | ||
| + System.currentTimeMillis() | ||
| + " ms with RefactoringException: Can't find java element to rename. "); |
There was a problem hiding this comment.
| "createRenameRefactoring finished in " | ||
| + System.currentTimeMillis() | ||
| + " ms with " | ||
| + renameRefactoring.toString()); |
There was a problem hiding this comment.
Signed-off-by: Dmytro Nochevnov <dnochevnov@codenvy.com>
This reverts commit 5e1ab4f. Signed-off-by: Dmytro Nochevnov <dnochevnov@codenvy.com>
Signed-off-by: Dmytro Nochevnov <dnochevnov@codenvy.com>
Signed-off-by: Dmytro Nochevnov <dnochevnov@codenvy.com>
dmytro-ndp
removed
the
status/code-review
…#8926) Signed-off-by: Dmytro Nochevnov <dnochevnov@codenvy.com>
What does this PR do?
Some selenium tests fails only on CI infrastructure in multithread mode. But reason of failing is not clear. We need more information for clarifying reasons. Storing javascript console of webdriver and sequence of requests/responses after fallen test can help for this
What issues does this PR fix or reference?
#8857
@dmytro-ndp @vparfonov @Ohrimenko1988