Adds in memory log storage, to be used while testing. #2858
Conversation
Enabling it only when config.debug is set, a configuration provided by jitsi-meet-torture.
|
just curious, what is this useful for? |
|
To get the console logs on test failures. Currently using the grid with chrome we have half of the logs, not sure why. So if the code is console.log("message", object), we get only "message". And all our logs are like that because of the Logger. And for Firefox, we got none as the remote logs retrieval is not implemented yet. |
|
I don't think it's "because of the Logger". It's just that the selenium driver only cares about the 1st argument. |
| @@ -121,6 +134,7 @@ function _libWillInit({ getState }, next, action) { | |||
| function _setLoggingConfig({ getState }, next, action) { | |||
| const result = next(action); | |||
| const newValue = getState()['features/base/logging'].config; | |||
| const isDebugEnabled = getState()['features/base/config'].debug; | |||
There was a problem hiding this comment.
would you mind using 'testing.testMode' instead of 'debug' ? I know torture was using 'debug' for some time already, but there's this dedicated 'testing' section in the config now.
|
Other than the config property it LGTM. If not the fact that Selenium does not support logs on FF I would probably prefer to concat all console.info arguments into string in an extra 'console' layer. Also I don't like that we are adding yet another usage of global APP variable, but I don't have any better idea at this point how to expose data to torture. |
| @@ -82,6 +87,16 @@ function _initLogging(loggingConfig) { | |||
| Logger.addGlobalTransport(APP.logCollector); | |||
| JitsiMeetJS.addGlobalLogTransport(APP.logCollector); | |||
| } | |||
|
|
|||
| if (isTestingEnabled) { | |||
There was a problem hiding this comment.
This section should be brought into the if above. It should not execute on mobile currently. There's no such thing as global "APP" there.
| /** | ||
| * Called by the <tt>LogCollector</tt> to store a series of log lines into | ||
| * batch. | ||
| * @param {string|object[]}logEntries an array containing strings |
There was a problem hiding this comment.
missing space between {string|object[]} and logEntries
| } | ||
|
|
||
| /** | ||
| * @return {boolean} <tt>true</tt> when this storage is ready or |
| const logEntry = logEntries[i]; | ||
|
|
||
| if (typeof logEntry === 'object') { | ||
| this.logs.push(logEntry.text); |
There was a problem hiding this comment.
where does logEntry.text come from ?
There was a problem hiding this comment.
oh I see it's an aggregated msg
Oh interesting, I just ran into the exact same issue on jibri. So does this get you the entire log message from our logger? I still only get the filename basically. The |
@paweldomas I wonder though if we can do something better in the logger though? |
Hmm that's interesting it's not what I saw. Will have to double check that |
|
Fixing it to show in chrome, will not fix Firefox, which doesn't have it implemented in the driver. |
* Adds in memory log storage, to be used while testing. Enabling it only when config.debug is set, a configuration provided by jitsi-meet-torture. * Moves to using config.testing.testMode property for logs storage. * Fixes comments.
* Adds in memory log storage, to be used while testing. Enabling it only when config.debug is set, a configuration provided by jitsi-meet-torture. * Moves to using config.testing.testMode property for logs storage. * Fixes comments.
Enabling it only when config.debug is set, a configuration provided by jitsi-meet-torture.