TestArmada / magellan

Large Scale Automated Testing for Mocha, Nightwatch, Appium, Nodejs, etc
MIT License
287 stars 76 forks source link

Add error message context for when saucelabs capactiy limit is reached #285

Closed g00dnatur3 closed 3 years ago

g00dnatur3 commented 3 years ago

TICKET: https://jira.walmart.com/browse/CEDXT-5046

This PR is to fix an error messaging issue.

A developer can be having their tests run in saucelabs, and alot of the time these tests can fail because of lack of saucelabs capacity.

Currently, when a saucelabs capacity error happens, the error message produced says the following:

13:38:19 Error retrieving a new session from the selenium server 13:38:19 Connection refused! Is selenium server started?

this causes confusion...

what is really happening behind the scene is saucelabs is sending a 500 with the details error message being a "capacity issue" - nightwatch catches a 500 and sends back a generic error message.

If we had tighter API and not CLI integration with nightwatch it would be possible to actually get the specific error message from saucelabs...

however, magellan executors are cli based and thus we have to work with what we have..

So this PR basically sniffs the stdout of the forked childprocess that is running the test...

and it scans for the "Connection refused! Is selenium server started?" error message...

if the error message occurs, we add more context to it, letting the user know he/she can just re-run the test if they're running on saucelabs cause its likely just a capacity issue...

maybe not a perfect solution -- but the juice is worth the squeeze with this approach...

getting the actual error message from saucelabs - would be uprooting and re-designing the magellan executors or something "BIG"

this is a kinda simple unobtrusive fix / workaround

Cheers!

this is the way the error message looks on the console BEFORE applying the fix:

Screen Shot 2020-07-27 at 2 47 38 PM

this is the way it looks AFTER applying the fix:

Screen Shot 2020-07-27 at 3 03 20 PM

[update] the above screenshot with type on "capacity" spelling has been fixed.

CLAassistant commented 3 years ago

CLA assistant check
All committers have signed the CLA.

codecov-commenter commented 3 years ago

Codecov Report

Merging #285 into master will increase coverage by 0.01%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #285      +/-   ##
==========================================
+ Coverage   97.24%   97.26%   +0.01%     
==========================================
  Files          31       31              
  Lines         981      988       +7     
==========================================
+ Hits          954      961       +7     
  Misses         27       27              
Impacted Files Coverage Δ
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 7978f53...a90062e. Read the comment docs.