firefox-devtools · jasonLaster · May 15, 2017 · May 9, 2017 · May 12, 2017 · May 15, 2017
diff --git a/circle.yml b/circle.yml
@@ -1,6 +1,6 @@
 machine:
   node:
-    version: 7.0
+    version: 7.7.2
   services:
     - docker
   environment:
@@ -20,7 +20,7 @@ test:
     - mkdir -p $CIRCLE_TEST_REPORTS/mocha
     - mkdir -p $CIRCLE_TEST_REPORTS/flow-coverage
     - mkdir -p $CIRCLE_TEST_REPORTS/jest-coverage
-    - node ./node_modules/.bin/jest --testResultsProcessor jest-junit-reporter --coverage --coverageDirectory=$CIRCLE_TEST_REPORTS/jest-coverage
+    - jest -i --testResultsProcessor jest-junit-reporter --coverage --coverageDirectory=$CIRCLE_TEST_REPORTS/jest-coverage
     - yarn license-check
     - yarn lint-css
     - yarn lint-js

diff --git a/flow-typed/debugger-html.js b/flow-typed/debugger-html.js
@@ -64,6 +64,20 @@ declare module "debugger-html" {
     actualLocation: Location
   };
 
+  /**
+ * PendingBreakpoint
+ *
+ * @memberof types
+ * @static
+ */
+  declare type PendingBreakpoint = {
+    location: Location,
+    loading: boolean,
+    disabled: boolean,
+    text: string,
+    condition: ?string
+  };
+
   /**
  * Frame ID
  *

diff --git a/package.json b/package.json
@@ -155,6 +155,7 @@
     ],
     "testPathIgnorePatterns": [
       "/node_modules/",
+      "/helpers/",
       "<rootDir>/test/",
       "<rootDir>/utils/tests/fixtures/"
     ],

diff --git a/src/actions/breakpoints.js b/src/actions/breakpoints.js
@@ -83,7 +83,6 @@ export function addBreakpoint(
         if (!text) {
           text = getTextForLine ? getTextForLine(actualLocation.line) : "";
         }
-
         return { id, actualLocation, text, hitCount };
       })()
     });
@@ -110,7 +109,7 @@ export function removeBreakpoint(location: Location) {
   return _removeOrDisableBreakpoint(location);
 }
 
-function _removeOrDisableBreakpoint(location, isDisabled) {
+function _removeOrDisableBreakpoint(location, isDisabled = false) {
   return ({ dispatch, getState, client }: ThunkArgs) => {
     let bp = getBreakpoint(getState(), location);
     if (!bp) {

diff --git a/src/actions/sources.js b/src/actions/sources.js
@@ -46,29 +46,38 @@ function checkSelectedSource(state, dispatch, source) {
   }
 }
 
-function checkPendingBreakpoints(state, dispatch, source) {
-  const pendingBreakpoints = getPendingBreakpoints(state);
-
-  if (pendingBreakpoints) {
-    pendingBreakpoints.forEach(pendingBreakpoint => {
-      const {
-        location: { line, sourceUrl, column },
-        condition
-      } = pendingBreakpoint;
-      const sameSource = sourceUrl && sourceUrl == source.url;
-
-      const location = { sourceId: source.id, sourceUrl, line, column };
+async function checkPendingBreakpoint(
+  state,
+  dispatch,
+  pendingBreakpoint,
+  source
+) {
+  const {
+    location: { line, sourceUrl, column },
+    condition
+  } = pendingBreakpoint;
+  const sameSource = sourceUrl && sourceUrl === source.url;
+  const location = { sourceId: source.id, sourceUrl, line, column };
+  const bp = getBreakpoint(state, location);
+
+  if (sameSource && !bp) {
+    if (location.column && isEnabled("columnBreakpoints")) {
+      await dispatch(addBreakpoint(location, { condition }));
+    } else {
+      await dispatch(addBreakpoint(location, { condition }));
+    }
+  }
+}
 
-      const bp = getBreakpoint(state, location);
+async function checkPendingBreakpoints(state, dispatch, source) {
+  const pendingBreakpoints = getPendingBreakpoints(state);
+  if (!pendingBreakpoints) {
+    return;
+  }
 
-      if (sameSource && !bp) {
-        if (location.column && isEnabled("columnBreakpoints")) {
-          dispatch(addBreakpoint(location, { condition }));
-        } else {
-          dispatch(addBreakpoint(location, { condition }));
-        }
-      }
-    });
+  const pendingBreakpointsArray = pendingBreakpoints.valueSeq().toJS();
+  for (let pendingBreakpoint of pendingBreakpointsArray) {
+    await checkPendingBreakpoint(state, dispatch, pendingBreakpoint, source);
   }
 }
 
@@ -78,23 +87,27 @@ function checkPendingBreakpoints(state, dispatch, source) {
  * @static
  */
 export function newSource(source: Source) {
-  return ({ dispatch, getState }: ThunkArgs) => {
+  return async ({ dispatch, getState }: ThunkArgs) => {
+    dispatch({ type: constants.ADD_SOURCE, source });
+
     if (prefs.clientSourceMapsEnabled) {
-      dispatch(loadSourceMap(source));
+      await dispatch(loadSourceMap(source));
     }
 
-    dispatch({ type: constants.ADD_SOURCE, source });
-
     checkSelectedSource(getState(), dispatch, source);
-    checkPendingBreakpoints(getState(), dispatch, source);
+    await checkPendingBreakpoints(getState(), dispatch, source);
   };
 }
 
 export function newSources(sources: Source[]) {
-  return ({ dispatch, getState }: ThunkArgs) => {
-    sources
-      .filter(source => !getSource(getState(), source.id))
-      .forEach(source => dispatch(newSource(source)));
+  return async ({ dispatch, getState }: ThunkArgs) => {
+    const filteredSources = sources.filter(
+      source => !getSource(getState(), source.id)
+    );
+
+    for (let source of filteredSources) {
+      await dispatch(newSource(source));
+    }
   };
 }
 

diff --git a/src/actions/tests/breakpoints.js b/src/actions/tests/breakpoints.js
@@ -1,34 +1,54 @@
-import { createStore, selectors, actions } from "../../utils/test-head";
 import expect from "expect.js";
 
-const simpleMockThreadClient = {
-  setBreakpoint: (location, condition) => {
-    return new Promise((resolve, reject) => {
-      resolve({ id: "hi", actualLocation: location });
-    });
-  },
-
-  removeBreakpoint: id => {
-    return new Promise((resolve, reject) => {
-      resolve({ status: "done" });
-    });
-  },
-
-  setBreakpointCondition: (id, location, condition, noSliding) => {
-    return new Promise((resolve, reject) => {
-      resolve({ sourceId: "a", line: 5 });
-    });
-  }
-};
+import { createStore, selectors, actions } from "../../utils/test-head";
+import {
+  simulateCorrectThreadClient,
+  simpleMockThreadClient
+} from "./helpers/breakpoints.js";
 
 describe("breakpoints", () => {
   it("should add a breakpoint", async () => {
     const { dispatch, getState } = createStore(simpleMockThreadClient);
+    const loc1 = { sourceId: "a", line: 5 };
 
-    await dispatch(actions.addBreakpoint({ sourceId: "a", line: 5 }));
-    await dispatch(actions.addBreakpoint({ sourceId: "b", line: 6 }));
+    await dispatch(actions.addBreakpoint(loc1));
+    const bps = selectors.getBreakpoints(getState());
+    const bp = selectors.getBreakpoint(getState(), loc1);
+    expect(bps.size).to.be(1);
+    expect(bp.location).to.eql(loc1);
+  });
 
-    expect(selectors.getBreakpoints(getState()).size).to.be(2);
+  it("should not re-add a breakpoint", async () => {
+    const { dispatch, getState } = createStore(simpleMockThreadClient);
+    const loc1 = { sourceId: "a", line: 5 };
+
+    await dispatch(actions.addBreakpoint(loc1));
+    let bps = selectors.getBreakpoints(getState());
+    const bp = selectors.getBreakpoint(getState(), loc1);
+    expect(bps.size).to.be(1);
+    expect(bp.location).to.eql(loc1);
+
+    await dispatch(actions.addBreakpoint(loc1));
+    bps = selectors.getBreakpoints(getState());
+    expect(bps.size).to.be(1);
+  });
+
+  describe("adding a breakpoint to an invalid location", async () => {
+    it("adds only one breakpoint with a corrected location", async () => {
+      const invalidLocation = { sourceId: "a", line: 5 };
+      const {
+        correctedThreadClient,
+        correctedLocation
+      } = simulateCorrectThreadClient(2, invalidLocation);
+      const { dispatch, getState } = createStore(correctedThreadClient);
+
+      await dispatch(actions.addBreakpoint(invalidLocation));
+      const state = getState();
+      const bps = selectors.getBreakpoints(state);
+      const bp = selectors.getBreakpoint(state, correctedLocation);
+      expect(bps.size).to.be(1);
+      expect(bp.location).to.eql(correctedLocation);
+    });
   });
 
   it("should remove a breakpoint", async () => {

diff --git a/src/actions/tests/helpers/breakpoints.js b/src/actions/tests/helpers/breakpoints.js
@@ -0,0 +1,79 @@
+import { makeLocationId } from "../../../reducers/breakpoints";
+
+export const theMockedPendingBreakpoint = {
+  location: {
+    sourceUrl: "http://localhost:8000/examples/bar.js",
+    line: 5,
+    column: undefined
+  },
+  condition: "3",
+  disabled: false
+};
+
+export function generateCorrectedBreakpoint(breakpoint, correctedLocation) {
+  return Object.assign({}, breakpoint, { location: correctedLocation });
+}
+
+function generateCorrectingThreadClient(offset = 0) {
+  return {
+    setBreakpoint: (location, condition) => {
+      const actualLocation = Object.assign({}, location, {
+        line: location.line + offset
+      });
+
+      return Promise.resolve({
+        id: makeLocationId(location),
+        actualLocation,
+        condition
+      });
+    }
+  };
+}
+
+/* in some cases, a breakpoint may be added, but the source will respond
+ * with a different breakpoint location. This is due to the breakpoint being
+ * added between functions, or somewhere that doesnt make sense. This function
+ * simulates that behavior.
+ * */
+export function simulateCorrectThreadClient(offset, location) {
+  const correctedThreadClient = generateCorrectingThreadClient(offset);
+  const offsetLine = { line: location.line + offset };
+  const correctedLocation = Object.assign({}, location, offsetLine);
+  return { correctedThreadClient, correctedLocation };
+}
+
+export function generateBreakpoint(filename) {
+  return {
+    location: {
+      sourceUrl: `http://localhost:8000/examples/${filename}`,
+      sourceId: filename,
+      line: 5
+    },
+    condition: null,
+    disabled: false
+  };
+}
+
+export function generatePendingBreakpoint(breakpoint) {
+  const {
+    location: { sourceUrl, line, column },
+    condition,
+    disabled
+  } = breakpoint;
+
+  return {
+    location: { sourceUrl, line, column },
+    condition,
+    disabled
+  };
+}
+
+export const simpleMockThreadClient = {
+  setBreakpoint: (location, _condition) =>
+    Promise.resolve({ id: "hi", actualLocation: location }),
+
+  removeBreakpoint: _id => Promise.resolve({ status: "done" }),
+
+  setBreakpointCondition: (_id, _location, _condition, _noSliding) =>
+    Promise.resolve({ sourceId: "a", line: 5 })
+};