Closed
Bug 1257570
Opened 9 years ago
Closed 8 years ago
--spsProfile doesn't work from try syntax
Categories
(Testing :: Talos, defect)
Testing
Talos
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.
Reporter | ||
Comment 1•9 years ago
|
||
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
Reporter | ||
Comment 2•8 years ago
|
||
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.
Flags: needinfo?(jmaher)
Comment 3•8 years ago
|
||
What tests are you concerned with? And are we mostly concerned with e10s or non-e10s here?
Reporter | ||
Comment 4•8 years ago
|
||
in this case regular non-e10s
Reporter | ||
Comment 5•8 years ago
|
||
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.
Reporter | ||
Updated•8 years ago
|
Flags: needinfo?(jmaher)
Reporter | ||
Comment 6•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/51085/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51085/
Attachment #8749649 -
Flags: review?(wlachance)
Updated•8 years ago
|
Attachment #8749649 -
Flags: review?(wlachance)
Comment 7•8 years ago
|
||
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)
Reporter | ||
Comment 8•8 years ago
|
||
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 9•8 years ago
|
||
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+
Comment 10•8 years ago
|
||
Comment 11•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment 12•8 years ago
|
||
This seems to be broken again. The tests run, but I don't get any profile data.
Comment 13•8 years ago
|
||
Might be related to bug 1331807? Probably worth filing a new bug.
Comment 14•8 years ago
|
||
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).
You need to log in
before you can comment on or make changes to this bug.
Description
•