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

Add intermittent failure reporting to filter-intermittents #16946

Merged
merged 3 commits into from Nov 7, 2017

Conversation

@jdm
Copy link
Member

jdm commented May 19, 2017

We need to address servo/saltfs#671 before this can be enabled by default on the builders, but I got the pieces working locally. This collects relevant information about each recorded intermittent failure encountered during a test run and posts them to a webapp which stores them in a database for later investigation. This will allow us to track over time which intermittent test failures are the most frequent, as well as whether they only happen on a particular builder or operating system.


This change is Reviewable

@highfive
Copy link

highfive commented May 19, 2017

Heads up! This PR modifies the following files:

@jdm
Copy link
Member Author

jdm commented May 19, 2017

@highfive highfive assigned Manishearth and unassigned mbrubeck May 19, 2017
data = {
'test_file': intermittent['test'],
'platform': 'TODO', # TODO: need saltfs changes
'builder': 'TODO', # TODO: need saltfs changes

This comment has been minimized.

@jdm

jdm May 19, 2017

Author Member

@aneeshusa What would be the best way to obtain the name of the builder here?

@jdm jdm force-pushed the jdm:report branch from 1f82dab to d55c2cf May 19, 2017
@Manishearth
Copy link
Member

Manishearth commented May 19, 2017

r=me

data = {
'test_file': intermittent['test'],
'platform': platform.system(),
'builder': 'TODO', # TODO: need saltfs changes

This comment has been minimized.

@aneeshusa

aneeshusa May 19, 2017

Member

Do you mean a Buildbot builder (e.g. android) or a build machine (servo-linux-cross3)?

This comment has been minimized.

@jdm

jdm May 19, 2017

Author Member

I mean build machine.

This comment has been minimized.

@aneeshusa

aneeshusa May 19, 2017

Member

I believe we'd want to expose the Buildbot slavename property as an environment variable by using interpolation, as mentioned in servo/saltfs#597.

(last_merge, _) = proc.communicate()

# Extract the issue reference from "abcdef Auto merge of #NNN"
pull_request = last_merge.split(' ')[4][1:]

This comment has been minimized.

@aneeshusa

aneeshusa May 19, 2017

Member

A bunch of this looks like it could be pulled out of the for intermittent in intermittents loop; I presume you'd want to add an early return if there are no intermittents as well.

@@ -556,6 +566,37 @@ def filter_intermittents(self, summary, log_filteredsummary, log_intermittents,
else:
intermittents += [failure]

if reporter_api:

This comment has been minimized.

@aneeshusa

aneeshusa May 19, 2017

Member

Are we setting this yet in buildbot_steps.yml?

This comment has been minimized.

@jdm

jdm May 19, 2017

Author Member

No. There's no point until I have an actual URL to give, which requires setting up the service first.

@nox
Copy link
Member

nox commented May 22, 2017

0.12s$ ./mach clean
Traceback (most recent call last):
  File "./mach", line 93, in <module>
    main(sys.argv)
  File "./mach", line 23, in main
    mach = mach_bootstrap.bootstrap(topdir)
  File "/home/travis/build/servo/servo/python/mach_bootstrap.py", line 270, in bootstrap
    mach.load_commands_from_file(os.path.join(topdir, path))
  File "/home/travis/build/servo/servo/python/_virtualenv/lib/python2.7/site-packages/mach/main.py", line 265, in load_commands_from_file
    imp.load_source(module_name, path)
  File "/home/travis/build/servo/servo/python/servo/testing_commands.py", line 544
    else if tracker_api.endswith('/'):
          ^
SyntaxError: invalid syntax
@bors-servo
Copy link
Contributor

bors-servo commented May 28, 2017

The latest upstream changes (presumably #17003) made this pull request unmergeable. Please resolve the merge conflicts.

@jdm
Copy link
Member Author

jdm commented Jun 14, 2017

@bors-servo: treeclosed=100

@jdm
Copy link
Member Author

jdm commented Jun 14, 2017

@bors-servo: treeclosed-

@jdm jdm force-pushed the jdm:report branch from d55c2cf to 5f3c4e6 Oct 18, 2017
@jdm jdm force-pushed the jdm:report branch 2 times, most recently from eba3e06 to 4893dd4 Oct 18, 2017
@jdm
Copy link
Member Author

jdm commented Oct 18, 2017

This needs servo/saltfs#739 to be merged and deployed before the API endpoint is available.

@jdm
Copy link
Member Author

jdm commented Oct 25, 2017

@highfive highfive assigned metajack and unassigned Manishearth Oct 25, 2017
@jdm
Copy link
Member Author

jdm commented Oct 25, 2017

Nevermind, Manish gave this r+ long ago.

@jdm jdm force-pushed the jdm:report branch from 4893dd4 to 50d641b Nov 7, 2017
@jdm jdm force-pushed the jdm:report branch from 50d641b to 86e3f0d Nov 7, 2017
@jdm
Copy link
Member Author

jdm commented Nov 7, 2017

@bors-servo: r=Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2017

📌 Commit 86e3f0d has been approved by Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2017

Testing commit 86e3f0d with merge 693c3dc...

bors-servo added a commit that referenced this pull request Nov 7, 2017
Add intermittent failure reporting to filter-intermittents

We need to address servo/saltfs#671 before this can be enabled by default on the builders, but I got the pieces working locally. This collects relevant information about each recorded intermittent failure encountered during a test run and posts them to a webapp which stores them in a database for later investigation. This will allow us to track over time which intermittent test failures are the most frequent, as well as whether they only happen on a particular builder or operating system.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16946)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2017

@bors-servo bors-servo merged commit 86e3f0d into servo:master Nov 7, 2017
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
You can’t perform that action at this time.