Skip to content

Rearrange jasmine test retries #3667

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Apr 1, 2019
Merged

Rearrange jasmine test retries #3667

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
34158c1
using different retry numbers for gl and flaky containers
archmoj Mar 22, 2019
c57d316
moved gl2d double-click into another file
archmoj Mar 24, 2019
5395282
Merge remote-tracking branch 'origin/master' into reduce-num-retries-…
archmoj Mar 29, 2019
8241bc8
reduced retries to 2 and 3 - marked one test as flaky
archmoj Mar 29, 2019
230544f
no foreach may help run the CI
archmoj Mar 29, 2019
17b5068
should reset one sankey test - no need to flaky flag as it fails loca…
archmoj Mar 29, 2019
4b0b824
Merge remote-tracking branch 'origin/master' into reduce-num-retries-…
archmoj Apr 1, 2019
305e0b3
set flaky retry number to 4 - before rerun 10 times
archmoj Apr 1, 2019
24aafc8
Apply Etienne recommendations
archmoj Apr 1, 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
6 changes: 3 additions & 3 deletions .circleci/test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ set +o pipefail

ROOT=$(dirname $0)/..
EXIT_STATE=0
MAX_AUTO_RETRY=5

log () {
echo -e "\n$1"
}

# inspired by https://unix.stackexchange.com/a/82602
MAX_AUTO_RETRY=1
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would we want to lower MAX_AUTO_RETRY?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks flaky tests (jasmine3) sometime required more retries. The gl tests on the other hand are slow and may not need that number of retries. Specially if they fail for a good reason, we don't want to wait too long to be notified that the test is actually failed.

Copy link
Contributor

Choose a reason for hiding this comment

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

The gl tests on the other hand are slow and may not need that number of retries.

Well, I suspect the gl tests are slow (now) because you reduced the number of test per shards to 1.

To me, having MAX_AUTO_RETRY=5 for all retry loops is the best of both world. Sufficient retry attempt, but failing runs that take to long to exit.

retry () {
local n=1

Expand Down Expand Up @@ -54,7 +54,7 @@ case $1 in
set_tz

SHARDS=($(node $ROOT/tasks/shard_jasmine_tests.js --tag=gl | circleci tests split))

MAX_AUTO_RETRY=2
for s in ${SHARDS[@]}; do
retry npm run test-jasmine -- "$s" --tags=gl --skip-tags=noCI --showSkipped
done
Expand All @@ -66,7 +66,7 @@ case $1 in
set_tz

SHARDS=($(node $ROOT/tasks/shard_jasmine_tests.js --tag=flaky | circleci tests split))

MAX_AUTO_RETRY=4
for s in ${SHARDS[@]}; do
retry npm run test-jasmine -- "$s" --tags=flaky --skip-tags=noCI --showSkipped
done
Expand Down
2 changes: 1 addition & 1 deletion tasks/shard_jasmine_tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ var argv = minimist(process.argv.slice(2), {
limit: ['l'],
},
default: {
limit: 20
limit: 1
Copy link
Contributor

Choose a reason for hiding this comment

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

So each test suite is runned separately?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. That's exactly what I wanted there for gl and flaky. I noticed e.g. setting timeout in one suite may have an effect on another suite. After having this in place the tests started to pass properly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you leave to default to 20 and pass a --limit 1 in the relevant test commands in .circleci/test.sh.

Do all npm run test-jasmine commands need to have --limit 1?

}
});

Expand Down
Loading