Closed Bug 1419326 Opened 7 years ago Closed 6 years ago

Run DAMP open/reload/close tests against a document specific to debugger

Categories

(DevTools :: Debugger, enhancement, P2)

Product:
DevTools
Component:
Debugger
Type:
enhancement

Tracking

(firefox59 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 59
Tracking Flags:
Tracking Status
firefox59 --- fixed

People

(Reporter: ochameau, Assigned: yulia)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Bug 1419326 - Run DAMP open/reload/close tests against a document specific to debugger
59 bytes, text/x-review-board-request
ochameau
: review+
Details 
Following bug 1415532, we should also run DAMP test against a custom document, made up for the debugger.
Severity: normal → enhancement
Priority: -- → P2
I've attached a WIP, there is an outstanding issue that the reload is not working as expected. Here is try so far: https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=f4af3153fd7c52065b77aeab5f157a30ecf1c7e2
Comment on attachment 8930428 [details]
Bug 1419326 - Run DAMP open/reload/close tests against a document specific to debugger

Discussed on irc about that. This patch looks good.
It looks like we are not ready to test sourcemapped sources as they fail reloading during page reload.
Better wait to fix that before testing that.

In the meantime we could already start adding a custom page stressing the debugger in other ways. Like testing with many js files/inline <script>/sync and async evals/async <script> creation/...
Attachment #8930428 - Flags: review?(poirot.alex) → feedback+
Attachment #8930428 - Flags: review?(poirot.alex) → review?(ystartsev)
Comment on attachment 8930428 [details]
Bug 1419326 - Run DAMP open/reload/close tests against a document specific to debugger

https://reviewboard.mozilla.org/r/201594/#review207848

::: testing/talos/talos/tests/devtools/addon/content/damp.js:523
(Diff revision 2)
> +
> +  async openDebuggerAndLog(label, expectedSources) {
> +    const waitForDebuggerSources = this.waitForDebuggerSources;
> +    const onLoad = async function(toolbox, dbg) {
> +      await waitForDebuggerSources(dbg, expectedSources);
>      };

No need for `waitForDebuggerSources` intermediate function here if you use an arrow function here:
const onLoad = async (toolbox, dbg) => {
  await this.waitForDebuggerSources(dbg, expectedSources);
};

(same comment about onReload)

::: testing/talos/talos/tests/devtools/addon/content/damp.js:544
(Diff revision 2)
> +    const expectedSources = 7;
>      let url = CUSTOM_URL.replace(/\$TOOL/, "debugger");
>      await this.testSetup(url);
> -    let toolbox = await this.openDebuggerAndLog(label);
> -    await this.reloadDebuggerAndLog(label, toolbox);
> -    await this.closeToolboxAndLog(label + ".jsdebugger");
> +    let toolbox = await this.openDebuggerAndLog(label, expectedSources);
> +    // TODO: add back in once https://github.com/devtools-html/debugger.html/pull/4769 is merged
> +    // await this.reloadDebuggerAndLog(label, toolbox, expectedSources);

What about testing some other random usecases in the meantime?
(and so drop the sourcemapped source and keep reloading the debugger)

Instead of only loading a sourcemapped source, you could:
* load many JS files
* load a big JS files (like copy of react-dom, or even much bigger)
* load many inline script
* load JS files asynchronously
* use eval
* ... anything that could possibly make the debugger slow.

You don't have to be exaustive in the first patch,
but at least cover one thing.

Also this sourcemapped file is very small, would it be enough to expose the regression you have about extra cost of sourcemaps?
Comment on attachment 8930428 [details]
Bug 1419326 - Run DAMP open/reload/close tests against a document specific to debugger

https://reviewboard.mozilla.org/r/201594/#review215000

Thanks a lot, looks good overall. You are the first to contribute a custom test page on DAMP :)
Do you have a link to a try push/perfherder for this?
It would be great to add a README or a comment somewhere in the custom document to explain how did you built this app. From which sources and with what commands.

Also, could you review each file you land in this example.
Is it any useful to have a favicon, a service worker, any css, two manifests (please keep it if that allows to rebuild the app from sources)? To be it looks like the only useful resources are the html page, the js file and its source map.

::: testing/talos/talos/tests/devtools/addon/content/damp.js:563
(Diff revision 3)
>      await this.reloadInspectorAndLog("custom", toolbox);
>      await this.closeToolboxAndLog("custom.inspector");
>      await this.testTeardown();
>    },
>  
> -  _getToolLoadingTests(url, label, { expectedMessages, expectedSources }) {
> +  waitForState(dbg, predicate, msg) {

Could you call this method waitForDebuggerState, or something that tells it is specific to debugger?
damp.js is full of random stuff, so it would help processing it.

::: testing/talos/talos/tests/devtools/addon/content/damp.js:572
(Diff revision 3)
> +        return resolve();
> +      }
> +
> +      const unsubscribe = dbg.store.subscribe(() => {
> +        if (predicate(dbg.store.getState())) {
> +          dump(`Finished waiting for state change: ${msg || ""}`);

Here and on line 565, you can assume you `msg` is mandatory, you always pass it.

::: testing/talos/talos/tests/devtools/addon/content/damp.js:586
(Diff revision 3)
> +    if (typeof url !== "string") {
> +      // Support passing in a source object itelf all APIs that use this
> +      // function support both styles
> +      const source = url;
> +      return source;
> +    }

I imagine you may have copied this function?
Here, you are always passing url string, so you can remove that. (You may notice I take care about not landing unused code)

::: testing/talos/talos/tests/devtools/addon/content/damp.js:606
(Diff revision 3)
> +
> +
> +  waitForDebuggerSources(dbg, expectedSources) {
> +    const { selectors, store } = dbg;
> +    function countSources() {
> +      const sources = selectors.getSources(store.getState());

You should use the state passed by `waitForSource` to `countSources`. Or remove the features from waitForSource (remove state passing to the `predicate` function)

::: testing/talos/talos/tests/devtools/addon/content/damp.js:611
(Diff revision 3)
> +      const sources = selectors.getSources(store.getState());
> +      return sources.size >= expectedSources;
> +    }
> +    if (countSources(store.getState())) {
> +      return Promise.resolve();
> +    }

These three lines look redundant with `waitForState`.

::: testing/talos/talos/tests/devtools/addon/content/damp.js:620
(Diff revision 3)
> +  async openDebuggerAndLog(label, expectedSources, file) {
> +   const onLoad = async (toolbox, dbgPanel) => {
> +    const dbg = await this.createDebuggerContext({}, dbgPanel);
> +    await this.waitForDebuggerSources(dbg, expectedSources);
> +    this.selectSource(dbg, file);
> +    await this.waitForSelectedSource(dbg, file);

nit: you may inline waitForSelectedSource call within selectSource.

::: testing/talos/talos/tests/devtools/addon/content/damp.js:645
(Diff revision 3)
> +          return false;
> +        }
> +
> +        if (!url) {
> +          return true;
> +        }

It looks like you always pass a url, so you should remove this.

::: testing/talos/talos/tests/devtools/addon/content/damp.js:653
(Diff revision 3)
> +        dump(`NEWSOURCE: ${newSource}`);
> +        if (newSource.id != source.get("id")) {
> +          return false;
> +        }
> +
> +        // wait for async work to be done

Could you elaborate a bit more in this comment?
It isn't clear what `hasSymbols` is/do.

::: testing/talos/talos/tests/devtools/addon/content/damp.js:661
(Diff revision 3)
> +      "selected source"
> +    );
> +  },
> +
> +  async createDebuggerContext(toolbox, dbgPanel) {
> +    const panel = dbgPanel || await toolbox.getPanelWhenReady("jsdebugger");

It should be easier to move
  await toolbox.getPanelWhenReady("jsdebugger");
to `reloadDebuggerAndLog` rather than having this complex logic here. createDebuggerContext would only accept a `panel` argument.

::: testing/talos/talos/tests/devtools/addon/content/damp.js:668
(Diff revision 3)
> +
> +    return {
> +      actions: actions,
> +      selectors: selectors,
> +      getState: store.getState,
> +      store: store

nit:
return {
  actions,
  selectors,
  getState: store.getState,
  store
};

::: testing/talos/talos/tests/devtools/addon/content/damp.js:686
(Diff revision 3)
> +
> +
> +  async customDebugger() {
> +    const label = "custom.debugger";
> +    const expectedSources = 7;
> +    let url = CUSTOM_URL.replace(/\$TOOL/, "debugger-sourcemaps/index");

I'm expecting to have only one document per tool is the `custom` folder, could you rename it to just `debugger`?
I'm expecting this document to not be only about source maps.

::: testing/talos/talos/tests/devtools/addon/content/damp.js:688
(Diff revision 3)
> +  async customDebugger() {
> +    const label = "custom.debugger";
> +    const expectedSources = 7;
> +    let url = CUSTOM_URL.replace(/\$TOOL/, "debugger-sourcemaps/index");
> +    await this.testSetup(url);
> +    const file = "App.js"

nit: it may be more explicit to name this variable:
selectedFile or selectedSource.
(also rename function arguments)
Attachment #8930428 - Flags: review?(poirot.alex)
Comment on attachment 8930428 [details]
Bug 1419326 - Run DAMP open/reload/close tests against a document specific to debugger

https://reviewboard.mozilla.org/r/201594/#review215358

asset-manifest.json and manifest.json are most likely not useful when running the test.

Looking at DAMP results:
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=mozilla-central&newProject=try&newRevision=0bfb28dbe5d299d9fd5ae9801d37cb4267bb90e0&originalSignature=edaec66500db21d37602c99daa61ac983f21a6ac&newSignature=edaec66500db21d37602c99daa61ac983f21a6ac&framework=1&selectedTimeRange=172800
It looks like you are not waiting for every asynchronous code to finish during reload (and may be open?)
  simple.reload goes from 124ms down to 53ms
  complicated.reload goes from 1.3s down to 452ms.
This is suspicious, we are most likely no longer waiting for everything during this step.

Here is how to debug that.
Apply this patch on top of yours:
diff --git a/testing/talos/talos/tests/devtools/addon/content/damp.js b/testing/talos/talos/tests/devtools/addon/content/damp.js
index eb4ae3e96537..4c6d147d790c 100644
--- a/testing/talos/talos/tests/devtools/addon/content/damp.js
+++ b/testing/talos/talos/tests/devtools/addon/content/damp.js
@@ -116,6 +116,8 @@ const debuggerHelper = {
 async function garbageCollect() {
   dump("Garbage collect\n");
 
+  await new Promise(done => setTimeout(done, 1000));
+
   // Minimize memory usage
   // mimic miminizeMemoryUsage, by only flushing JS objects via GC.
   // We don't want to flush all the cache like minimizeMemoryUsage,
@@ -654,6 +656,7 @@ async _consoleOpenWithCachedMessagesTest() {
       await debuggerHelper.selectSource(dbg, selectedFile);
     }
     await this.reloadPageAndLog(`${label}.debugger`, onReload);
+    await garbageCollect();
   },
 
 

This will add a pause after each test and especially after "reload" test. So that you can look at the profiler, go to the Marker Chabert in order to see what executes right after each test. Each test generates one marker, like "complicated.jsdebugger.reload". You can select a timeframe in the timeline, right after one test marker and see what executes.

In theory, nothing should execute after a test. In most of the existing test there is still some pending reflows or GCs, but we should do our best to prevent having any pending Javascript still running.

Here is an example of such profile:
  https://perfht.ml/2lOfaZY
And the same profile focused on what executes right after complicated.reload:
  https://perfht.ml/2lLGZ5l
Same again, but focused after complicated.open:
  https://perfht.ml/2lL3iYJ
In both cases, we can see some call(s) to debugger.js's dispatch method followed by many reflows.

::: testing/talos/talos/tests/devtools/addon/content/damp.js:80
(Diff revision 4)
> +      const sources = selectors.getSources(state);
> +      return sources.size >= expectedSources;
> +    }
> +    if (countSources(dbg.getState())) {
> +      return Promise.resolve();
> +    }

You can remove these 3 lines as waitForState is going to do that.

::: testing/talos/talos/tests/devtools/addon/content/damp.js:108
(Diff revision 4)
> +        const source = dbg.selectors.getSelectedSource(state);
> +        const isLoaded = source && source.get("loadedState") === "loaded";
> +        if (!isLoaded) {
> +          return false;
> +        }
> +        // wait for async work to be done

Could you say in this comment why we have to call `hasSymbols` -or- tell us what it does?

::: testing/talos/talos/tests/devtools/addon/content/damp.js:656
(Diff revision 4)
> +      const panel = await toolbox.getPanelWhenReady("jsdebugger")
> +      const dbg = await debuggerHelper.createContext(panel);
> +      await debuggerHelper.waitForSources(dbg, expectedSources);
> +      await debuggerHelper.selectSource(dbg, selectedFile);
> +    }
> +    await this.reloadPageAndLog(`${label}.debugger`, onReload);

s/debugger/jsdebugger/

Historically, debugger tool internal name/ID is "jsdebugger".
Here, you end up changing the test name and we would loose history for the "reload" test.

You can see that on DAMP result page
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=mozilla-central&newProject=try&newRevision=0bfb28dbe5d299d9fd5ae9801d37cb4267bb90e0&originalSignature=edaec66500db21d37602c99daa61ac983f21a6ac&newSignature=edaec66500db21d37602c99daa61ac983f21a6ac&framework=1&selectedTimeRange=172800
Attachment #8930428 - Flags: review?(poirot.alex)
I made the updates as requested, but i struggled a bit with getting all of the js calls accounted for

What is happening is that after the loading of the text, we have a line flashing to show which location is currently selected. This shouldn't happen on initial load, so it is a bug. but it is also happening outside of the bounds of the damp test. I tried to compensate for this by listening to the codemirror updates with a small delay, but this means that a delay will be introduced to every test. this isn't great but I couldn't think of something better. I am open to suggestions

here is the perf for the current state of this branch (but with the added delay of 1000 ms to make it easier to see what is going on):
https://perfht.ml/2EXNfzy

and here is the try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0bfb28dbe5d299d9fd5ae9801d37cb4267bb90e0

I am not too sure how to run a talos run remotely, it feels risky to do -t all. what is the recommended way to do this?
> I am not too sure how to run a talos run remotely, it feels risky to do -t
> all. what is the recommended way to do this?

There is document about DAMP over here:
http://docs.firefox-dev.tools/tests/performance-tests.html
-t g2-10s is the key to run DAMP tests. It isn't really clear what you mean with "run a talos run remotely".
You try push ran DAMP. What more do you need?

Unfortunately, your try run still highlights some impact on the next tests: styleeditor.
Today, I landed bug 1426688, which should help waiting for pending reflows.
This is not ideal. Ideally the test would wait for everything, but that would help prevent polluting the following tests.

Could you rebase against latest mozilla-central and repush to try?
Once your try run is completed, compare against this try run:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=fd7cd00254cedeb741cd63b0d819025a57dab570
As this just landed, comparing against last 2 days of mozilla-central will report weak results as the timing of various tests changed.
To do that, on PerfHerder compare page, select "Try" in the "Project" menulist and check the "Compare with a specific revision" checkbox, then you can put fd7cd00254cedeb741cd63b0d819025a57dab570 as base revision to compare with.

> What is happening is that after the loading of the text, we have a line
> flashing to show which location is currently selected. This shouldn't happen
> on initial load, so it is a bug. but it is also happening outside of the
> bounds of the damp test. I tried to compensate for this by listening to the
> codemirror updates with a small delay, but this means that a delay will be
> introduced to every test. this isn't great but I couldn't think of something
> better. I am open to suggestions

The arbitrary delay looks bad, but I have nothing else to suggest, I just don't know debugger codebase enough.
Try removing the delay while rebasing, to see if everyhing ends in the new "settle" subtests.
Otherwise, you could move the delay out of the "reload" subtest.
You would totally ignore all these computations for now and come back to it later in a followup.
So that, at least, it doesn't pollute the tests running after debugger.
sure, I removed the polling for codemirror, and rebased. I am now just confused what -t none is doing, since its not disabling talos tests. 

try run is here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=99c06cfe0353b2a4c63cef8011bc917ab60ea7c4

I will check again once it finishes
Assignee: nobody → ystartsev
Comment on attachment 8930428 [details]
Bug 1419326 - Run DAMP open/reload/close tests against a document specific to debugger

https://reviewboard.mozilla.org/r/201594/#review216636

::: testing/talos/talos/tests/devtools/addon/content/damp.js:710
(Diff revision 8)
> +   const onLoad = async (toolbox, panel) => {
> +    const dbg = await debuggerHelper.createContext(panel);
> +    await debuggerHelper.waitForSources(dbg, expectedSources);
> +    await debuggerHelper.selectSource(dbg, selectedFile);
> +    await debuggerHelper.waitForText(dbg, selectedFile, expectedText);
> +   };

This `onLoad` method doesn't wait for the little spider icon displayed on the left of "App.js" tab.

It seems to be the only visual update you miss on toolbox opening. There may still be some fade out of the first line within codemirror, but that sounds less important.

::: testing/talos/talos/tests/devtools/addon/content/damp.js:721
(Diff revision 8)
> +    const onReload = async () => {
> +      const panel = await toolbox.getPanelWhenReady("jsdebugger")
> +      const dbg = await debuggerHelper.createContext(panel);
> +      await debuggerHelper.waitForSources(dbg, expectedSources);
> +      await debuggerHelper.waitForText(dbg, selectedFile, expectedText);
> +    }

This is not doing what you expect. It seems to resolve immediately as the sources and text is already set to the expected values during toolbox opening.
Before these assertions, you should wait for some event related to reload. For example, when you reload, the source text editor disappear.

I don't know what is the best event to listen for, but with this patch, reload test seems to behave more correctly. But we still miss the spider icon update and codemirror syntax highlighting.
```
diff --git a/testing/talos/talos/tests/devtools/addon/content/damp.js b/testing/talos/talos/tests/devtools/addon/content/damp.js
index dbb67fb19574..4c55d9fab85f 100644
--- a/testing/talos/talos/tests/devtools/addon/content/damp.js
+++ b/testing/talos/talos/tests/devtools/addon/content/damp.js
@@ -716,6 +716,17 @@ async _consoleOpenWithCachedMessagesTest() {
 
   async reloadDebuggerAndLog(label, toolbox, expectedSources, selectedFile, expectedText) {
     const onReload = async () => {
+      await new Promise(done => {
+        let count = 0;
+        let { client } = toolbox.target;
+        let onSource = async (_, actor) => {
+          if (++count >= 1) {
+            client.removeListener("newSource", onSource);
+            done();
+          }
+        };
+        client.addListener("newSource", onSource);
+      });
       const panel = await toolbox.getPanelWhenReady("jsdebugger")
       const dbg = await debuggerHelper.createContext(panel);
       await debuggerHelper.waitForSources(dbg, expectedSources);
```

In order to debug such issue I record screenshots synchronously right after test resolves. With such snippet:
```
let window = toolbox.win;
let canvas = window.document.createElementNS("http://www.w3.org/1999/xhtml", "html:canvas");
let context = canvas.getContext("2d");
canvas.width = window.innerWidth;
canvas.height = window.innerHeight;
context.drawWindow(window, 0, 0, canvas.width, canvas.height, "white");
this._win.gBrowser.addTab(canvas.toDataURL());
```

It may also help to do:
```
  await new Promise(()=>{});
```
in order to freeze the test at some point to compare your screenshot versus how debugger ends up.

I just pushed this on try:
https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=743ec8b8785bba5a85529db3578de30071465bfd
Attachment #8930428 - Flags: review?(poirot.alex)
Thanks alex, i think you found the issue with waiting for the icon -- what was going on is that the metadata was being parsed, and for most tests this doesnt have any visual impact. I forgot about this and was focusing only on what could be seen.

let me know if everything is being caught now

Try run: https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=3e80e845e9fa071844c70735839fa60f6ef66d85
Comment on attachment 8930428 [details]
Bug 1419326 - Run DAMP open/reload/close tests against a document specific to debugger

https://reviewboard.mozilla.org/r/201594/#review217056

::: testing/talos/talos/tests/devtools/addon/content/damp.js:737
(Diff revisions 8 - 9)
>        const panel = await toolbox.getPanelWhenReady("jsdebugger")
>        const dbg = await debuggerHelper.createContext(panel);
>        await debuggerHelper.waitForSources(dbg, expectedSources);
>        await debuggerHelper.waitForText(dbg, selectedFile, expectedText);
> +      await debuggerHelper.waitForMetaData(dbg);
>      }

This `onReload` method still resolves immediately and makes custom.jsdebugger.reload be smaller than expected. It resolves immediately because onReload is called almost immediately, and debugger UI isn't yet cleaned up and so the sources is still displayed after toolbox opening.
Attachment #8930428 - Flags: review?(poirot.alex)
https://searchfox.org/mozilla-central/source/testing/talos/talos/tests/devtools/addon/content/damp.js#126
`onReload` method you defined in `reloadDebuggerAndLog` is called here.
It is called immediately, before `browser.reload()`, so it will resolve immediately as it is the same code than `onLoad` which just resolved during toolbox open test.

I think it is hard to see via vidyo such timing detail, but I tried logging the screenshot at end of onReload, like you did, and I also get the screenshot logged immediately, as we just started reloading the page. You can see the debugger UI still updating while the screenshot tab is already opened.

When we reload the page, we reset the source tree and close the source editor. Is there some events fired when doing that?
If yes, you could listen for such event from onReload and you should fix this issue.
Thanks for catching that -- the solution I came up with was to listen for the navigate event, which fires every time the debugger reloads or navigates. this seems to fix the issue

try run: https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=ba443f0a84fa11ff857d4e6f85751c66fad53169
Comment on attachment 8930428 [details]
Bug 1419326 - Run DAMP open/reload/close tests against a document specific to debugger

https://reviewboard.mozilla.org/r/201594/#review217214

Looks good locally, and also on PerfHerder. Thanks Yulia!

https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=mozilla-central&newProject=try&newRevision=ba443f0a84fa11ff857d4e6f85751c66fad53169&originalSignature=edaec66500db21d37602c99daa61ac983f21a6ac&newSignature=edaec66500db21d37602c99daa61ac983f21a6ac&filter=jsdebugger&framework=1&selectedTimeRange=172800
You improved debugger test is many very important ways:
* the coverage is much larger. You cover source tree, source editor and propably many other things we were totally blind from DAMP perspective!
* the noise of new test is fantasticly low! 0.2% for complicated open and reload, this is really great as it should help us identify regressions as low as 0.4%.
* your test do not leak asynchronous code. jsdebugger.open.settle and jsdebugger.reload.settle are around zero. It means that you wait for everything correctly. There is custom.reload.settle which is at 14ms. I imagine it is codemirror fade out of the first line. But this is not critical to fix that.

::: testing/talos/talos/tests/devtools/addon/content/damp.js:132
(Diff revision 10)
> +        const state = dbg.store.getState();
> +        const source = dbg.selectors.getSelectedSource(state);
> +        // wait for metadata -- this involves parsing the file to determine its type.
> +        // if the object is empty, the data has not yet loaded
> +        const metaData = dbg.selectors.getSourceMetaData(state, source.get("id"));
> +        dump(`objects: ${Object.keys(metaData)}\n`);

This message spams the stdout, may be we should remove it?

Waiting until: has file metadata
objects:
objects:
objects:
...
objects: isReactComponent
Finished Waiting until: has file metadata

::: testing/talos/talos/tests/devtools/addon/content/damp.js:769
(Diff revision 10)
> +    let url = CUSTOM_URL.replace(/\$TOOL/, "debugger/index");
> +    await this.testSetup(url);
> +    const selectedFile = "App.js";
> +    const expectedText = "import React, { Component } from 'react';";
> +    const toolbox = await this.openDebuggerAndLog(label, expectedSources, selectedFile, expectedText);
> +    await this.reloadDebuggerAndLog(label, toolbox, expectedSources, selectedFile, expectedText);

You should pass "custom" instead of `label` (="custom.jsdebugger").
You are already appending "jsdebugger" within `reloadDebuggerAndLog` and `openDebuggerAndLog`.
But this is not done within `closeToolboxAndLog`, so you should still pass `label` to this one.
Attachment #8930428 - Flags: review?(poirot.alex) → review+
made the update! almost missed the closeToolboxAndLog, thanks for mentioning it.
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f8beb72039b2
Run DAMP open/reload/close tests against a document specific to debugger r=ochameau
I updated to fix the eslint changes on my changes, but i noticed that there were some other eslint issues in other parts of the testing directory. is this ok?
Flags: needinfo?(ystartsev) → needinfo?(dluca)
(In reply to Yulia Startsev [:yulia] from comment #34)
> I updated to fix the eslint changes on my changes, but i noticed that there
> were some other eslint issues in other parts of the testing directory. is
> this ok?

It looks good, I ran eslint locally and it is green now.
Flags: needinfo?(dluca)
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8dc35c769853
Run DAMP open/reload/close tests against a document specific to debugger r=ochameau
https://hg.mozilla.org/mozilla-central/rev/8dc35c769853
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Push from comment 37 updated DAMP perf baseline:

== Change summary for alert #11154 (as of Thu, 11 Jan 2018 16:51:01 GMT) ==

Improvements:

 10%  damp osx-10-10 opt e10s     108.95 -> 97.67
 10%  damp windows10-64 pgo e10s  86.66 -> 77.88
 10%  damp windows7-32 opt e10s   111.60 -> 100.31
  9%  damp windows10-64 opt e10s  95.90 -> 87.02
  9%  damp windows7-32 pgo e10s   84.09 -> 76.60
  5%  damp linux64 pgo e10s       79.47 -> 75.73

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=11154
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: