Closed Bug 1257570 Opened 5 years ago Closed 5 years ago

--spsProfile doesn't work from try syntax

Categories

(Testing :: Talos, defect)

defect
Not set
normal

Tracking

(firefox49 fixed)

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: jmaher, Unassigned)

References

Details

Attachments

(1 file)

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.
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
What tests are you concerned with? And are we mostly concerned with e10s or non-e10s here?
in this case regular non-e10s
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.
Flags: needinfo?(jmaher)
Attachment #8749649 - Flags: review?(wlachance)
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 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/
Attachment #8749649 - Flags: review?(wlachance)
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
Attachment #8749649 - Flags: review?(wlachance) → review+
https://hg.mozilla.org/mozilla-central/rev/47bc46e92a73
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
This seems to be broken again. The tests run, but I don't get any profile data.
Might be related to bug 1331807? Probably worth filing a new bug.
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).
Blocks: 1333167
Blocks: 1354964
You need to log in before you can comment on or make changes to this bug.