Bug 1363678 - Allow to pass a custom logLimit to configureStore. r=bgrins draft
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Thu, 01 Jun 2017 10:23:51 +0200
changeset 587670 23b86faa00d08a80f2cb21291c0df3b4c8113e73
parent 587553 381c1335cb67c17c98922b089235889b06bbc6ec
child 631341 8c529bea17aa67f739d183fc87825d0f3d2c2b92
push id61784
push userbmo:nchevobbe@mozilla.com
push dateThu, 01 Jun 2017 14:30:43 +0000
reviewersbgrins
bugs1363678
milestone55.0a1
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());