Skip to content
This repository was archived by the owner on Jan 19, 2021. It is now read-only.

Conversation

@g00dnatur3
Copy link
Contributor

@g00dnatur3 g00dnatur3 commented Aug 24, 2020

Currently the "ERROR" and "WARN" logs from nightwatch are suppressed when the debug flag is turned OFF..
and our customers ALWAYS use the debug off, because if you turn it on your log will be filled with base64 screenshot gobbledegook...

Also, the approach i took, we will see all the "ERROR-ed" selenium request/response logs in our magellan log

BEFORE LOG

Screen Shot 2020-08-24 at 3 42 39 PM

AFTER LOG

Screen Shot 2020-08-24 at 3 37 24 PM

in the example above i modified the nightwatch code in my node_modules to have this code Logger.warn('__EXAMPLE_WORKING__ Success saving screenshot 200 ok');

Previously, this code would not have been logged to the magellan stdout.

in the AFTER LOG you can see that it is working GREAT :)

the â is really the red X .. its just my terminal settings are making it show up as â instead of the fancy X

@codecov-commenter
Copy link

codecov-commenter commented Aug 24, 2020

Codecov Report

Merging #289 into master will decrease coverage by 0.27%.
The diff coverage is 81.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #289      +/-   ##
==========================================
- Coverage   97.26%   96.99%   -0.28%     
==========================================
  Files          31       31              
  Lines         988      997       +9     
==========================================
+ Hits          961      967       +6     
- Misses         27       30       +3     
Impacted Files Coverage Δ
src/cli.js 93.40% <25.00%> (-1.54%) ⬇️
src/util/childProcess.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 54824f2...9b5dd73. Read the comment docs.

Copy link
Contributor

@chiahrens chiahrens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

src/cli.js Outdated
rootWorkingDirectory: process.cwd()
});
}
if (settings.framework === 'testarmada-magellan-nightwatch-plugin') {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we were just gonna pass this in as a command-line arg? This is sorta ugly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the whole idea it to make this the "new" default..

we are not being fully verbose in our magellan log, in the child process we are filtering out the "verbose" stuff and only shinning a light on the errors and warns

so this should be the default, its the whole point...

opts.argv.debug = true. just tells niughtwatch to be verbose...

i guess we can say if the (opts.argv.debug) is already true then we dont filter out anything in the child process...

you know what im saying?

const STDOUT_WHITE_LIST = ['ERROR', 'WARN', 'Test Suite', '✖']

// we slice the VERBOSE nighwatch stdout stream on the purple INFO text that has black background
const SLICE_ON_TEXT = '\033[1;35m\033[40mINFO\033[0m'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you know if this works for both nightwatch .9 and 1.x?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good question.

looking at their logger in the master:
https://github.com/nightwatchjs/nightwatch/blob/master/lib/utils/logger.js

it seems it should work just fine cause they havent changed the logger INFO coloring at all

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
}

isTextWhiteListed(text) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return STDOUT_WHITE_LIST.some(item => text.includes(item))

Suggested change
isTextWhiteListed(text) {
isTextWhiteListed(text) {
return STDOUT_WHITE_LIST.some(item => text.includes(item))
}
}

?

@g00dnatur3 g00dnatur3 merged commit 2ff9431 into master Aug 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants