Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

chore: fix tests and CI flakiness #16806

Closed
wants to merge 8 commits into from
Closed

chore: fix tests and CI flakiness #16806

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
302ce17
test(ngHref): only run Edge-specific test on relevant Edge versions
gkalpak Jan 3, 2019
993a445
chore(saucelabs): upgrade to latest SauceConnect
gkalpak Jan 3, 2019
ac73fce
chore(saucelabs): use 'websocket' for transmission when possible
gkalpak Jan 8, 2019
a99ebed
chore(package): upgrade to latest `karma`
gkalpak Jan 3, 2019
c57688a
chore(package): upgrade to latest `karma-sauce-launcher`
gkalpak Jan 3, 2019
119bbd0
chore(travis): suppress verbose log output and allow error logging
gkalpak Jan 3, 2019
ac459f8
chore(saucelabs): add `SL_EDGE-1` to browser list
gkalpak Jan 8, 2019
b96d334
chore(saucelabs): remove `SL_Safari` from browser list
gkalpak Jan 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions karma-shared.conf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,7 @@ module.exports = function(config, specificOptions) {
// SauceLabs config for local development.
sauceLabs: {
testName: specificOptions.testName || 'AngularJS',
startConnect: true,
options: {
// We need selenium version +2.46 for Firefox 39 and the last selenium version for OS X is 2.45.
// TODO: Uncomment when there is a selenium 2.46 available for OS X.
// 'selenium-version': '2.46.0'
}
startConnect: true
},

// BrowserStack config for local development.
Expand Down Expand Up @@ -189,6 +184,9 @@ module.exports = function(config, specificOptions) {
config.sauceLabs.tunnelIdentifier = process.env.TRAVIS_JOB_NUMBER;
config.sauceLabs.recordScreenshots = true;

// Try 'websocket' for a faster transmission first. Fallback to 'polling' if necessary.
config.transports = ['websocket', 'polling'];

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this not the default?

Copy link
Member Author

Choose a reason for hiding this comment

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

According to the docs, the default is ['polling', 'websocket'] and it is not clear if ordering makes a difference, so I thought it wouldn't hurt to match angular/angular#27634.
But, yeah, this might be redundant.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that socket.io will

By default, a long-polling connection is established first, then upgraded to “better” transports (like WebSocket).

So if I understand it, ['polling', 'websocket'] would connect with polling initially and then try to connect via a websocket. Seems strange but I guess that polling comes up quicker for short-lived connections?

Copy link
Member Author

Choose a reason for hiding this comment

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

🤷‍♂️

// Debug logging into a file, that we print out at the end of the build.
config.loggers.push({
type: 'file',
Expand Down
15 changes: 6 additions & 9 deletions lib/saucelabs/start_tunnel.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ set -e
# Curl and run this script as part of your .travis.yml before_script section:
# before_script:
# - curl https://gist.github.com/santiycr/5139565/raw/sauce_connect_setup.sh | bash
SC_VERSION="4.5.1"
SC_VERSION="4.5.2"
CONNECT_URL="https://saucelabs.com/downloads/sc-$SC_VERSION-linux.tar.gz"
CONNECT_DIR="/tmp/sauce-connect-$RANDOM"
CONNECT_DOWNLOAD="sc-$SC_VERSION-linux.tar.gz"

CONNECT_LOG="$LOGS_DIR/sauce-connect"
CONNECT_STDOUT="$LOGS_DIR/sauce-connect.stdout"
CONNECT_STDERR="$LOGS_DIR/sauce-connect.stderr"
# We don't want to create a log file because sauceconnect always logs in verbose mode. This seems
# to be overwhelming Travis and causing flakes when we are cat-ing the log in "print_logs.sh"
CONNECT_LOG="/dev/null"
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting!

Copy link
Member Author

Choose a reason for hiding this comment

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

TBH, I don't know if it was actually affecting flakiness. I was just porting bits that looked useful from Angular PRs. (This came from angular/angular#27657.)


# Get Connect and start it
mkdir -p $CONNECT_DIR
Expand All @@ -42,9 +42,6 @@ if [ ! -z "$BROWSER_PROVIDER_READY_FILE" ]; then
fi


echo "Starting Sauce Connect in the background, logging into:"
echo " $CONNECT_LOG"
echo " $CONNECT_STDOUT"
echo " $CONNECT_STDERR"
echo "Starting Sauce Connect in the background"
sauce-connect/bin/sc -u $SAUCE_USERNAME -k $SAUCE_ACCESS_KEY $ARGS \
--logfile $CONNECT_LOG 2> $CONNECT_STDERR 1> $CONNECT_STDOUT &
--logfile $CONNECT_LOG &
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
"jquery": "3.2.1",
"jquery-2.1": "npm:[email protected]",
"jquery-2.2": "npm:[email protected]",
"karma": "3.1.1",
"karma": "^3.1.4",
"karma-browserstack-launcher": "^1.3.0",
"karma-chrome-launcher": "^2.2.0",
"karma-edge-launcher": "^0.4.2",
Expand All @@ -71,7 +71,7 @@
"karma-jasmine": "^1.1.2",
"karma-junit-reporter": "^1.2.0",
"karma-safari-launcher": "^1.0.0",
"karma-sauce-launcher": "^1.2.0",
"karma-sauce-launcher": "^2.0.2",
"karma-script-launcher": "^1.0.0",
"karma-spec-reporter": "^0.0.32",
"load-grunt-tasks": "^3.5.0",
Expand Down
8 changes: 4 additions & 4 deletions scripts/travis/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ export SAUCE_ACCESS_KEY
BROWSER_STACK_ACCESS_KEY=$(echo "$BROWSER_STACK_ACCESS_KEY" | rev)
SAUCE_ACCESS_KEY=$(echo "$SAUCE_ACCESS_KEY" | rev)

# TODO: restore "SL_EDGE-1" once Sauce Labs adds Edge 17 and "SL_EDGE-1" refers
# to version 16. Edge 15 disconnects from Karma frequently causing extreme build instability.
# The currently latest version of Safari on Saucelabs (v12) is unstable and disconnects frequently.
# TODO: Add `SL_Safari` back, once/if it becomes more stable again.
BROWSERS="SL_Chrome,SL_Chrome-1,\
SL_Firefox,SL_Firefox-1,\
SL_Safari,SL_Safari-1,\
SL_Safari-1,\
SL_iOS,SL_iOS-1,\
SL_IE_9,SL_IE_10,SL_IE_11,\
SL_EDGE"
SL_EDGE,SL_EDGE-1"

case "$JOB" in
"ci-checks")
Expand Down
2 changes: 1 addition & 1 deletion scripts/travis/print_logs.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ for FILE in $LOG_FILES; do
echo "================================================================================"
echo " $FILE"
echo "================================================================================"
cat $FILE
cat $FILE || true
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this cat fail?

Copy link
Member Author

Choose a reason for hiding this comment

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

If there are no log files, then for FILE in $LOG_FILES uses $LOGS_DIR/* literally, which does not exist and thus cat fails.

done
4 changes: 2 additions & 2 deletions test/ng/directive/ngHrefSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ describe('ngHref', function() {
}));


// Support: IE 9-11 only, Edge 12-15+
if (msie || /\bEdge\/[\d.]+\b/.test(window.navigator.userAgent)) {
// Support: IE 9-11 only, Edge 12-17
if (msie || /\bEdge\/1[2-7]\.[\d.]+\b/.test(window.navigator.userAgent)) {
// IE/Edge fail when setting a href to a URL containing a % that isn't a valid escape sequence
// See https://github.com/angular/angular.js/issues/13388
it('should throw error if ng-href contains a non-escaped percent symbol', inject(function($rootScope, $compile) {
Expand Down
Loading