Skip to content
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.

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

retry eyes test failures only when non-eyes errors occur #19659

Merged
merged 7 commits into from Dec 15, 2017

Conversation

davidsbailey
Copy link
Member

@davidsbailey davidsbailey commented Dec 14, 2017

Background

the following call tree illustrates roughly how test flakiness is computed:

  • dashboard/test/ui/runner.rb
    • cucumber
      • eyes_steps.rb → @eyes.close(throw_exception = true)
      • connect.rb → post success status to saucelabs rest api
  • bin/test_flakiness → get success status from saucelabs rest api

"magic retry" uses test flakiness data to decide how many times to rerun each UI test feature. the goal is to enable this for eyes tests during DTTs (but not on Circle), only counting selenium errors (not eyes diffs) as flaky failures.

Description

  1. in eyes_steps.rb
    (a) make eyes mismatches no longer cause the cucumber run to fail, causing these runs to be reported as successful in terms of test flakiness (described briefly above).
    (b) print eyes mismatch errors to the cucumber log
  2. in runner.rb
    (a) parse the cucumber output to detect eyes mismatches
    (b) fail the test run if eyes mismatch is detected
  3. in test.rake, turn on "magic retry" for eyes tests

Screenshots

The following looks different when the only eyes failures occur:

in slack (note "1 eyes mismatches")
screen shot 2017-12-14 at 12 13 12 am

on the command line (note "1 eyes mismatches"):
screen shot 2017-12-13 at 8 36 39 pm

in the cucumber logs (note red warning text and linkified eyes link):
screen shot 2017-12-13 at 11 54 16 pm

Testing

Manually verified locally and on the test machine:

  • features with only selenium errors show no change in behavior
  • passing features show no change in behavior
  • features with selenium errors AND eyes mismatches appear just like features with only selenium errors

@islemaster
Copy link
Contributor

👁️‍🗨️ Reading now....

Copy link
Contributor

@islemaster islemaster left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for doing this Dave!

# Note that applitools strips the trailing dot on this url for us, so we
# don't need to strip it here.
def linkify(str)
str.to_s.gsub(/(https?:\/\/[^ ]+)/, '<a href="\1">\1</a>')
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd replace [^ ] with \S so any whitespace ends the link. Alternatively, there are a number of gems for this sort of thing.

begin
@eyes.close(fail_on_mismatch)
rescue Applitools::TestFailedError => e
puts "<span style=\"color: red;\">#{EYES_ERROR_PREFIX} #{linkify(e)}</span>"
Copy link
Contributor

Choose a reason for hiding this comment

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

Which log does this puts go to?

Copy link
Member Author

Choose a reason for hiding this comment

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

This html ends up in the cucumber log output, e.g. including:

  1. the html output (e.g. ChromeLatestWin7_angleHelper_eyes_output.html), and
  2. the stdout stream (which is consumed by runner.rb, and may also be what you'd end up with in success.log if you didn't specify runner.rb --html).

@@ -8,6 +8,10 @@
# See http://support.applitools.com/customer/en/portal/articles/2099488-match-timeout
MATCH_TIMEOUT = 5

# Prefix to print before any eyes errors. We depend on this in runner.rb to detect
# whether there were any eyes errors.
EYES_ERROR_PREFIX = '[Applitools Eyes error]'
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we load this constant from a shared location in both eyes_steps.rb and runner.rb?

Copy link
Member Author

Choose a reason for hiding this comment

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

but then I'd have to pick a name for the file this gets extracted into :-) PTAL

Copy link
Contributor

@islemaster islemaster left a comment

Choose a reason for hiding this comment

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

I think you picked a great name 😁 LGTM!

@davidsbailey davidsbailey merged commit eb150b0 into staging Dec 15, 2017
@davidsbailey davidsbailey deleted the rerun-non-eyes-failures branch December 15, 2017 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants