1257570 - --spsProfile doesn't work from try syntax

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

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

on march 15th, mconley was successful in pushing to try and getting sps profiles:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=034a6b40d711

I tried a day later and it failed:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=feb9b7b467b2

I am not sure why this failed, but we should make this more foolproof.

Comment 1

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

here is the code that does the parsing of the try syntax to enable spsProfiling:
https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/testing/talos.py#167

Comment 2

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

another successful push for mconley:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a41d19779b06fc263907eeb62bb40bc95717c7cc&selectedJob=19685857

and a failed one for bholley:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1757eed493c0&selectedJob=20428313

ok, now I tried a push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4f4ca84b9004

self.ni? to follow up tonight or tomorrow.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

What tests are you concerned with? And are we mostly concerned with e10s or non-e10s here?

Comment 4

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

in this case regular non-e10s

Comment 5

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

ok, if the comment is multi line, the parsing fails, which my push is, and bholley's push, but mconley has a single line comment.

Comment 6

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

Attachment: MozReview Request: Bug 1257570 - --spsProfile doesn't work from try syntax. r?wlach

Review commit: https://reviewboard.mozilla.org/r/51085/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51085/

Comment 7

[William Lachance (:wlach)]

Comment on attachment 8749649 [details]
MozReview Request: Bug 1257570 - --spsProfile doesn't work from try syntax. r?wlach

https://reviewboard.mozilla.org/r/51085/#review47741

Awesome! There are some minor revisions I'd like to see before this goes in.

::: testing/mozharness/mozharness/mozilla/testing/talos.py:177
(Diff revision 1)
>              # now let's see if we added spsProfile specs in the commit message
>              try:
>                  junk, junk, opts = self.buildbot_config['sourcestamp']['changes'][-1]['comments'].partition('mozharness:')
> +
> +                # we need to guard against multi line comments
> +                opts = opts.split('\n')[0]

You should put this lower, like line 183, since it's not strictly part of the try...except clause.

I'd also make the comment something like:

```py
# In the case of a multi-line commit message, only examine the first line for mozharness options
```

(you might need to split that across 2 lines)

Comment 8

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

Comment on attachment 8749649 [details]
MozReview Request: Bug 1257570 - --spsProfile doesn't work from try syntax. r?wlach

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/51085/diff/1-2/

Comment 9

[William Lachance (:wlach)]

Comment on attachment 8749649 [details]
MozReview Request: Bug 1257570 - --spsProfile doesn't work from try syntax. r?wlach

https://reviewboard.mozilla.org/r/51085/#review47781

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/47bc46e92a73

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/47bc46e92a73

Comment 12

[Kris Maglione [:kmag]]

This seems to be broken again. The tests run, but I don't get any profile data.

Comment 13

[Bobby Holley (:bholley)]

Might be related to bug 1331807? Probably worth filing a new bug.

Comment 14

[Kris Maglione [:kmag]]

I don't think so. From the logs, it doesn't look like the sps profile is even being recorded. On Linux, anyway. On Windows, the tests fail to run, but it looks like it is trying to record profile data (which I think is bug 1295644).