Bug 1363678 - Allow to pass a custom logLimit to configureStore. r=bgrins
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Thu, 01 Jun 2017 10:23:51 +0200
changeset 409948 39d218ebd0d6c3d0e677d22b860485aabc77c592
parent 409947 f481ef5c3bf0496828b1c645cfdf9f029c895f2d
child 409949 a3bdbdd4d95a22b3cdc190d2a62ac3cdcc2addde
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1363678
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1363678 - Allow to pass a custom logLimit to configureStore. r=bgrins The idea is to allow tests to set their own logLimit without having to use Services.setIntPref. To do so, we also need to not retrieve the logLimit from Services in the messages reducer. Since we already have the logLimit in the store, we can retrieve it directly from the store by passing the prefs state to the messages reducer. We take this as an opportunity to revisit the createRootReducer function to be future proof: if any new reducer is added, they should just work as if we were using combineReducers. With those changes, we can set a lower logLimit in some tests that used to have a defined timeout since they were taking too much time. MozReview-Commit-ID: 7j80XoKkJ1y
devtools/client/webconsole/new-console-output/reducers/messages.js
devtools/client/webconsole/new-console-output/store.js
devtools/client/webconsole/new-console-output/test/helpers.js
devtools/client/webconsole/new-console-output/test/store/messages.test.js
--- a/devtools/client/webconsole/new-console-output/reducers/messages.js
+++ b/devtools/client/webconsole/new-console-output/reducers/messages.js
@@ -12,19 +12,16 @@ const constants = require("devtools/clie
 const {isGroupType} = require("devtools/client/webconsole/new-console-output/utils/messages");
 const {
   MESSAGE_TYPE,
   MESSAGE_SOURCE
 } = require("devtools/client/webconsole/new-console-output/constants");
 const { getGripPreviewItems } = require("devtools/client/shared/components/reps/reps");
 const { getSourceNames } = require("devtools/client/shared/source-utils");
 
-const Services = require("Services");
-const logLimit = Math.max(Services.prefs.getIntPref("devtools.hud.loglimit"), 1);
-
 const MessageState = Immutable.Record({
   // List of all the messages added to the console.
   messagesById: Immutable.OrderedMap(),
   // Array of the visible messages.
   visibleMessages: [],
   // List of the message ids which are opened.
   messagesUiById: Immutable.List(),
   // Map of the form {messageId : tableData}, which represent the data passed
@@ -35,26 +32,28 @@ const MessageState = Immutable.Record({
   groupsById: Immutable.Map(),
   // Message id of the current group (no corresponding console.groupEnd yet).
   currentGroup: null,
   // List of removed messages is used to release related (parameters) actors.
   // This array is not supposed to be consumed by any UI component.
   removedMessages: [],
 });
 
-function messages(state = new MessageState(), action, filtersState) {
+function messages(state = new MessageState(), action, filtersState, prefsState) {
   const {
     messagesById,
     messagesUiById,
     messagesTableDataById,
     groupsById,
     currentGroup,
     visibleMessages,
   } = state;
 
+  const {logLimit} = prefsState;
+
   switch (action.type) {
     case constants.MESSAGE_ADD:
       let newMessage = action.message;
 
       if (newMessage.type === constants.MESSAGE_TYPE.NULL_MESSAGE) {
         // When the message has a NULL type, we don't add it.
         return state;
       }
@@ -104,17 +103,17 @@ function messages(state = new MessageSta
 
         if (shouldMessageBeVisible(addedMessage, record, filtersState)) {
           record.set("visibleMessages", [...visibleMessages, newMessage.id]);
         }
 
         // Remove top level message if the total count of top level messages
         // exceeds the current limit.
         if (record.messagesById.size > logLimit) {
-          limitTopLevelMessageCount(state, record);
+          limitTopLevelMessageCount(state, record, logLimit);
         }
       });
 
     case constants.MESSAGES_CLEAR:
       return new MessageState({
         // Store all removed messages associated with some arguments.
         // This array is used by `releaseActorsEnhancer` to release
         // all related backend actors.
@@ -239,17 +238,17 @@ function getParentGroups(currentGroup, g
   return groups;
 }
 
 /**
  * Remove all top level messages that exceeds message limit.
  * Also populate an array of all backend actors associated with these
  * messages so they can be released.
  */
-function limitTopLevelMessageCount(state, record) {
+function limitTopLevelMessageCount(state, record, logLimit) {
   let topLevelCount = record.groupsById.size === 0
     ? record.messagesById.size
     : getToplevelMessageCount(record);
 
   if (topLevelCount <= logLimit) {
     return record;
   }
 
--- a/devtools/client/webconsole/new-console-output/store.js
+++ b/devtools/client/webconsole/new-console-output/store.js
@@ -17,21 +17,22 @@ const {
   MESSAGES_CLEAR,
   REMOVED_MESSAGES_CLEAR,
   BATCH_ACTIONS,
   PREFS,
 } = require("devtools/client/webconsole/new-console-output/constants");
 const { reducers } = require("./reducers/index");
 const Services = require("Services");
 
-function configureStore(hud) {
+function configureStore(hud, options = {}) {
+  const logLimit = options.logLimit
+    || Math.max(Services.prefs.getIntPref("devtools.hud.loglimit"), 1);
+
   const initialState = {
-    prefs: new PrefState({
-      logLimit: Math.max(Services.prefs.getIntPref("devtools.hud.loglimit"), 1),
-    }),
+    prefs: new PrefState({ logLimit }),
     filters: new FilterState({
       error: Services.prefs.getBoolPref(PREFS.FILTER.ERROR),
       warn: Services.prefs.getBoolPref(PREFS.FILTER.WARN),
       info: Services.prefs.getBoolPref(PREFS.FILTER.INFO),
       debug: Services.prefs.getBoolPref(PREFS.FILTER.DEBUG),
       log: Services.prefs.getBoolPref(PREFS.FILTER.LOG),
       css: Services.prefs.getBoolPref(PREFS.FILTER.CSS),
       net: Services.prefs.getBoolPref(PREFS.FILTER.NET),
@@ -46,26 +47,31 @@ function configureStore(hud) {
     createRootReducer(),
     initialState,
     compose(applyMiddleware(thunk), enableActorReleaser(hud), enableBatching())
   );
 }
 
 function createRootReducer() {
   return function rootReducer(state, action) {
-    const newFiltersState = reducers.filters(state.filters, action);
-    return Object.assign({}, {
-      filters: newFiltersState,
-      prefs: reducers.prefs(state.prefs, action),
-      ui: reducers.ui(state.ui, action),
-      // specifically pass the updated filters state as an additional argument.
+    // We want to compute the new state for all properties except "messages".
+    const newState = [...Object.entries(reducers)].reduce((res, [key, reducer]) => {
+      if (key !== "messages") {
+        res[key] = reducer(state[key], action);
+      }
+      return res;
+    }, {});
+
+    return Object.assign(newState, {
+      // specifically pass the updated filters and prefs state as additional arguments.
       messages: reducers.messages(
         state.messages,
         action,
-        newFiltersState
+        newState.filters,
+        newState.prefs,
       ),
     });
   };
 }
 
 /**
  * A enhancer for the store to handle batched actions.
  */
--- a/devtools/client/webconsole/new-console-output/test/helpers.js
+++ b/devtools/client/webconsole/new-console-output/test/helpers.js
@@ -26,18 +26,18 @@ function setupActions() {
   };
 
   return wrappedActions;
 }
 
 /**
  * Prepare the store for use in testing.
  */
-function setupStore(input, hud) {
-  const store = configureStore(hud);
+function setupStore(input, hud, options) {
+  const store = configureStore(hud, options);
 
   // Add the messages from the input commands to the store.
   input.forEach((cmd) => {
     store.dispatch(actions.messageAdd(stubPackets.get(cmd)));
   });
 
   return store;
 }
--- a/devtools/client/webconsole/new-console-output/test/store/messages.test.js
+++ b/devtools/client/webconsole/new-console-output/test/store/messages.test.js
@@ -163,19 +163,18 @@ describe("Message reducer:", () => {
       expect(visibleMessages[logLimit - 1].parameters[0]).toBe(`message-${logLimit - 1}`);
 
       // The groups were cleaned up.
       const groups = getAllGroupsById(getState());
       expect(groups.count()).toBe(0);
     });
 
     it("properly limits number of groups", () => {
-      const { dispatch, getState } = setupStore([]);
-
-      const logLimit = 1000;
+      const logLimit = 100;
+      const { dispatch, getState } = setupStore([], null, {logLimit});
 
       const packet = clonePacket(stubPackets.get("console.log(undefined)"));
       const packetGroup = clonePacket(stubPackets.get("console.group('bar')"));
       const packetGroupEnd = clonePacket(stubPackets.get("console.groupEnd()"));
 
       for (let i = 0; i < logLimit + 2; i++) {
         dispatch(actions.messageAdd(packetGroup));
         packet.message.arguments = [`message-${i}-a`];
@@ -191,22 +190,21 @@ describe("Message reducer:", () => {
       expect(messages.count()).toBe(logLimit * 3);
 
       // We should have logLimit number of groups
       const groups = getAllGroupsById(getState());
       expect(groups.count()).toBe(logLimit);
 
       expect(visibleMessages[1].parameters[0]).toBe(`message-2-a`);
       expect(messages.last().parameters[0]).toBe(`message-${logLimit + 1}-b`);
-    }).timeout(5000);
+    });
 
     it("properly limits number of collapsed groups", () => {
-      const { dispatch, getState } = setupStore([]);
-
-      const logLimit = 1000;
+      const logLimit = 100;
+      const { dispatch, getState } = setupStore([], null, {logLimit});
 
       const packet = clonePacket(stubPackets.get("console.log(undefined)"));
       const packetGroupCollapsed = clonePacket(
         stubPackets.get("console.groupCollapsed('foo')"));
       const packetGroupEnd = clonePacket(stubPackets.get("console.groupEnd()"));
 
       for (let i = 0; i < logLimit + 2; i++) {
         packetGroupCollapsed.message.arguments = [`group-${i}`];
@@ -228,17 +226,17 @@ describe("Message reducer:", () => {
 
       expect(messages.first().parameters[0]).toBe(`group-2`);
       expect(messages.last().parameters[0]).toBe(`message-${logLimit + 1}-b`);
 
       const visibleMessages = getVisibleMessages(getState());
       expect(visibleMessages.length).toBe(logLimit);
       const lastVisibleMessage = visibleMessages[visibleMessages.length - 1];
       expect(lastVisibleMessage.parameters[0]).toBe(`group-${logLimit + 1}`);
-    }).timeout(5000);
+    });
 
     it("does not add null messages to the store", () => {
       const { dispatch, getState } = setupStore([]);
 
       const message = stubPackets.get("console.time('bar')");
       dispatch(actions.messageAdd(message));
 
       const messages = getAllMessagesById(getState());