Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Automated perf tests #10517

Merged
merged 1 commit into from
Oct 18, 2017
Merged

Automated perf tests #10517

merged 1 commit into from
Oct 18, 2017

Conversation

ayumi
Copy link
Contributor

@ayumi ayumi commented Aug 16, 2017

#10115

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.

Test Plan:

Reviewer Checklist:

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

.travis.yml Outdated
@@ -37,3 +38,5 @@ addons:
packages:
- xvfb
- g++-4.8
- libgnome-keyring-dev
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just remove this dependency in #10514. Do we need it back?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, was a mis-rebase. good catch!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@ayumi ayumi force-pushed the feature/perf-tests branch 2 times, most recently from bb777f1 to fbf731e Compare August 18, 2017 07:40
.travis.yml Outdated
@@ -1,6 +1,6 @@
language: node_js
node_js:
- "7"
- "8"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be updated as well?

browser-laptop/package.json

Lines 230 to 232 in 836d391

"engines": {
"node": ">= 7.9.0"
}

@NejcZdovc
Copy link
Contributor

@ayumi are you working on this one right now? 😃 @bsclifton said that you have some problems with travis, so I wanted to check it out and help out

@ayumi
Copy link
Contributor Author

ayumi commented Sep 4, 2017

Currently blocked on fixing --debug on Linux:
brave/muon#292

@ayumi ayumi force-pushed the feature/perf-tests branch 8 times, most recently from b160b7b to 45fafbd Compare September 27, 2017 18:42
@ayumi ayumi force-pushed the feature/perf-tests branch 2 times, most recently from d860bb4 to fa476c9 Compare September 28, 2017 03:58
@luixxiul
Copy link
Contributor

luixxiul commented Oct 8, 2017

Is it not worth adding a large amount of cache to see how much it affects the performance?

@ayumi
Copy link
Contributor Author

ayumi commented Oct 10, 2017

@luixxiul what do you mean?

this is no longer blocked on linux muon; i'm working around the --debug linux crash by using muon debug builds rather than using the prod builds.

@luixxiul
Copy link
Contributor

I meant loading a cache file to calculate how much it can slow down the browser.

@ayumi ayumi force-pushed the feature/perf-tests branch 3 times, most recently from dcdcbc4 to ffbc9c1 Compare October 12, 2017 02:31
@cezaraugusto
Copy link
Contributor

@ayumi is this ready for review? please put the work-in-progress label otherwise, thanks

@cezaraugusto cezaraugusto added the needs-info Another team member needs information from the PR/issue opener. label Oct 14, 2017
@ayumi
Copy link
Contributor Author

ayumi commented Oct 14, 2017

@cezaraugusto yes, this is ready for review

the tests have been reasonably deterministic.

@bsclifton bsclifton added this to the 0.22.x (Nightly Channel) milestone Oct 17, 2017
@bsclifton
Copy link
Member

@ayumi I'm trying to test this out and can use some help 😄

  1. Would you be able to create some docs to accompany this? Maybe a new wiki entry:
    https://github.com/brave/browser-laptop/wiki

  2. I checked out this branch on an Ubuntu 14.04 VM, removed node_modules, and re-did the npm install. After that completed, I ran the following:

curl -sL https://raw.githubusercontent.com/travis-ci/artifacts/master/install | bash
CXX=g++-4.8 NODE_ENV=test TEST_DIR=performance ARTIFACTS_REGION=us-east-1 npm run testsuite

When running this, I'm getting a few errors (some tests are timing out). However, it all seems great on the Travis side (which is more important 😄 ):
https://travis-ci.org/brave/browser-laptop/jobs/286826649

Any advice on what to try the setup and test for?

  1. You had shown me the Grafana instance (would be good to document some details about that). This is uploading the results of a run to S3. Once merged, this should be collecting all of the perf data about each PR, correct?

Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments left 😄 I think most (if not all) of the concerns I had can be solved with documentation. The actual changes look great! It's really exciting seeing the progress you made. Great job! 😄 👍

@bsclifton
Copy link
Member

BTW- per discussion on Slack, the --debug option should work on Linux builds for the latest Muon. If you get a chance, please retry that and let us know how it works (if resolved, we can close brave/muon#292). Until we get a chance to test that, we can still keep the separate debug version of Muon 😄

@ayumi
Copy link
Contributor Author

ayumi commented Oct 17, 2017

  1. 👍

  2. travis artifacts uploader isn't needed outside of travis

  3. Yes, after this is merged we will start collecting data on every Travis run for browser-laptop; and it will automatically go to the https://github.com/brave/perfaderp web UI.

  4. --debug still doesn't work for me on Linux. browser-laptop master 3ee0c9543f582794117b3ce98832da339285429d

Uploads cpu profiles to S3.
@bsclifton
Copy link
Member

@ayumi the last commit has some failing tests. I restarted the travis job- if that passes, this looks great; will merge 😄

@ayumi
Copy link
Contributor Author

ayumi commented Oct 18, 2017

@bsclifton I might need to create a new muon debug build. however, I can update it without code changes (just by uploading the builds to S3)

@bsclifton
Copy link
Member

@ayumi OK great, sounds good 😄 Let's merge this for now and you can investigate that 😄

@bsclifton bsclifton merged commit 11f16cf into master Oct 18, 2017
@bsclifton bsclifton deleted the feature/perf-tests branch October 18, 2017 20:05
@ayumi
Copy link
Contributor Author

ayumi commented Oct 19, 2017

tests working again with a new muon debug build

@bbondy bbondy modified the milestones: 0.22.x (Developer Channel), 0.23.x (Nightly Channel) Feb 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automated-tests needs-info Another team member needs information from the PR/issue opener. perf
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants