775610 - Talos tests should always fail if __FAIL is encountered

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

See bug 775102. If the log for a talos test has both of the tokens __FAIL and __start_report, the test is considered to pass. I think this behaviour should be modified so that anytime the __FAIL token is found in the log, the test is automatically flagged as failing. This makes it easier to catch failures which might otherwise be ignored.

Comment 1

[Jeff Hammel]

Do you happen to have a log handy where this occurs?

Comment 2

[Geoff Brown [:gbrown]]

Most (all?) of the robocop-based Talos tests currently report:

NOISE: __FAILchecking that painted surface loaded: painted surface loaded__FAIL

yet most of those are green. One example: https://tbpl.mozilla.org/php/getParsedLog.php?id=13780257&tree=Mozilla-Inbound&full=1

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

interesting, we parse the results and only in the case of an exception do we look for fail.

:jhammel, would you have any reservations to change this to look for fail first, then parse the results?

i.e. change results.py:
        # parse the results
        try:
            self.parse()
        except utils.talosError:
            # see we can find a reason for the failure
            match = self.RESULTS_REGEX_FAIL.search(self.results_raw)
            if match:
                self.error(match.group(1))
                raise utils.talosError(match.group(1))
            raise # reraise failing exception


to look more like:
        # parse the results
        try:
            match = self.RESULTS_REGEX_FAIL.search(self.results_raw)
            if match:
                self.error(match.group(1))
                raise utils.talosError(match.group(1))
            self.parse()
        except utils.talosError:
            raise # reraise failing exception

Comment 4

[Jeff Hammel]

:jmaher, yeah that looks like a good fix.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: talos should fail if we see __FAIL (1.0)

patch as discussed in bug comments.

Comment 6

[Jeff Hammel]

Comment on attachment 645354 [details] [diff] [review]
talos should fail if we see __FAIL (1.0)

lgtm

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

http://hg.mozilla.org/build/talos/rev/e5ef20b8c71d

Comment 8

[Kartikaya Gupta (email:[e-mail])]

We should fix bug 775614 before this goes into production, or things will start failing.

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

start fixing!