Victor Porof [:vporof][:vp] Reporter
	Description • 9 years ago

Meta bug to track intermittents that need to be looked at.

	Victor Porof [:vporof][:vp] Reporter
	Comment 1 • 8 years ago

I'm sorry

	Victor Porof [:vporof][:vp] Reporter
	Comment 2 • 8 years ago

Cleaned up events.js a little bit.

	Jordan Santell [:jsantell] [@jsantell]
	Comment 3 • 8 years ago

Comment on attachment 8728510 [details] [diff] [review]
v2

Review of attachment 8728510 [details] [diff] [review]:
-----------------------------------------------------------------

Wow. Looks great. Overall, r+ -- I looked at everything, but if anything needs a closer look, let me know. For the currently disabled tests, let's do follow ups and I can help out too, and get this giant patch landed. Should be ok as we're not landing any other changes to this yet.

VERY EXCITING

::: devtools/client/performance/performance-controller.js
@@ -266,5 @@
> -    // Emit another stop event here, as a lot of tests use
> -    // the RECORDING_STOPPED event, but in the case of a UI click on a button,
> -    // the RECORDING_STOPPED event happens from the server, where this request may
> -    // not have yet finished, so listen to this in tests that fail because the `stopRecording`
> -    // request is not yet completed. Should only be used in that scenario.

Oh my god I forgot about this

@@ +416,5 @@
>     * Stores a recording internally.
>     *
>     * @param {PerformanceRecordingFront} recording
>     */
> +  _addRecordingIfUnknown: function (recording) {

Not a fan of this method name, but I don't care too much.

::: devtools/client/performance/test/browser.ini
@@ +31,5 @@
> +[browser_perf-details-04-toolbar-buttons.js]
> +[browser_perf-details-05-preserve-view.js]
> +[browser_perf-details-06-rerender-on-selection.js]
> +[browser_perf-details-07-bleed-events.js]
> +# [browser_perf-details-gc-snap.js] TODO

Add bug number bug 1161817

@@ -83,5 @@
>  [browser_perf-overview-selection-02.js]
>  [browser_perf-overview-selection-03.js]
>  [browser_perf-overview-time-interval.js]
>  [browser_perf-private-browsing.js]
> -skip-if = os == 'linux' # bug 1210140

These work on Linux now? nice

::: devtools/client/performance/test/browser_perf-docload.js
@@ +22,2 @@
>    yield startRecording(panel);
> +  yield reload(target);

way better

::: devtools/client/performance/test/head.js
@@ +10,5 @@
> +// Performance tests are much heavier because of their reliance on the
> +// profiler module, memory measurements, frequent canvas operations etc. Many of
> +// of them take longer than 30 seconds to finish on try server VMs, even though
> +// they superficially do very little.
> +requestLongerTimeout(2);

Yuck -- is there any downside to this if a test does not need to take longer?

@@ +45,5 @@
> +
> +  DevToolsUtils.testing = true;
> +
> +  // Make sure all the prefs are reverted to their defaults once tests finish.
> +  let stopObservingPrefs = PrefUtils.whenUnknownPrefChanged("devtools.performance", pref => {

Nice

::: devtools/client/performance/test/helpers/event-utils.js
@@ +1,1 @@
> +/* Any copyright is dedicated to the Public Domain.

Most of these could probably live in devtools/client/framework/test/shared-head, or maybe a new devtools/client/testing/*.js modules, as these seem pretty handy to be used elsewhere. (Follow up)

	Victor Porof [:vporof][:vp] Reporter
	Comment 4 • 8 years ago

(In reply to Jordan Santell [:jsantell] [@jsantell] (Please needinfo) from comment #3)
> 
> ::: devtools/client/performance/test/browser.ini
> @@ +31,5 @@
> > +[browser_perf-details-04-toolbar-buttons.js]
> > +[browser_perf-details-05-preserve-view.js]
> > +[browser_perf-details-06-rerender-on-selection.js]
> > +[browser_perf-details-07-bleed-events.js]
> > +# [browser_perf-details-gc-snap.js] TODO
> 
> Add bug number bug 1161817

Will file a new bug for re-enabling these.

> 
> ::: devtools/client/performance/test/head.js
> @@ +10,5 @@
> > +// Performance tests are much heavier because of their reliance on the
> > +// profiler module, memory measurements, frequent canvas operations etc. Many of
> > +// of them take longer than 30 seconds to finish on try server VMs, even though
> > +// they superficially do very little.
> > +requestLongerTimeout(2);
> 
> Yuck -- is there any downside to this if a test does not need to take longer?
> 

I don't see any downsides other than accidentally writing long tests. But we already have this issue, as most of our tests take longer than usual to finish.

	Victor Porof [:vporof][:vp] Reporter
	Comment 5 • 8 years ago

Final cleanup, added more assertions and listening to more events for the console tests. Landing.

	Victor Porof [:vporof][:vp] Reporter
	Comment 6 • 8 years ago

https://hg.mozilla.org/integration/fx-team/rev/220192149ce4f79c75611295db062f8fd5933b8b
Bug 1245886 - Refactor performance tests, r=jsantell

	Victor Porof [:vporof][:vp] Reporter
	Comment 7 • 8 years ago

Added commit message and landed: https://hg.mozilla.org/integration/fx-team/rev/220192149ce4

Final try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4a00b4aea8e&group_state=expanded

	Victor Porof [:vporof][:vp] Reporter
	Comment 8 • 8 years ago

https://hg.mozilla.org/integration/fx-team/rev/4116cafbb87dd251990450f8f90911beecbe0859
Bug 1245886 - Force GC at the end of all tests, r=me

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 9 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/220192149ce4

	Victor Porof [:vporof][:vp] Reporter
	Comment 10 • 8 years ago

https://hg.mozilla.org/integration/fx-team/rev/8671dfbbff2dfa2fa6c410f4c0799f4b7c2e7484
Bug 1245886 - Manually stop the profiler module at the end of all tests, r=me

	Carsten Book [:Tomcat]
	Comment 11 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/4116cafbb87d

	Victor Porof [:vporof][:vp] Reporter
	Comment 12 • 8 years ago

Triaging. Filter on ADRENOCORTICOTROPIC (yes).

	Carsten Book [:Tomcat]
	Comment 13 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/8671dfbbff2d

	Ryan VanderMeulen [:RyanVM]
	Comment 14 • 8 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-aurora/rev/9539d658b1a9
https://hg.mozilla.org/releases/mozilla-aurora/rev/bc74c5ca31af
https://hg.mozilla.org/releases/mozilla-aurora/rev/60828c17117a

	Victor Porof [:vporof][:vp] Reporter
	Comment 15 • 8 years ago

Can't work on this right now, busy with Tofino.

	BugBot [:suhaib / :marco/ :calixte]
	Comment 16 • 6 years ago

The leave-open keyword is there and there is no activity for 6 months.
:julienw, maybe it's time to close this bug?

	Julien Wajsberg [:julienw]
	Comment 17 • 6 years ago

We do have some intermittent bugs but we have other ways to track them (especially being bugged about them every week, but also through keywords). Let's close this bug.