Currently: 156 warnings in total. Goal: 0.

Comment on attachment 8630338 [details] [diff] [review] Fix all warnings in Loop shared test suite Review of attachment 8630338 [details] [diff] [review]: ----------------------------------------------------------------- This is almost there, I'd just like to give it another once over when you've addressed the comments. ui-showcase.jsx and standalone/index.html both needs their counts updating. ::: browser/components/loop/test/shared/activeRoomStore_test.js @@ +19,5 @@ > sandbox.useFakeTimers(); > > dispatcher = new loop.Dispatcher(); > sandbox.stub(dispatcher, "dispatch"); > + closeWindowStub = sandbox.stub(window, "close"); I don't think we need the specific stub names here. Although its an alias, something like sinon.assert.calledOnce(window.close) is pretty obvious its been stubbed, and imho its easier to work out at a glance. @@ +1185,5 @@ > + > + it("should log an error in the console", function() { > + listener(new Error("foo")); > + > + sinon.assert.calledOnce(errorStub); ditto here with the errorStub name. ::: browser/components/loop/test/shared/conversationStore_test.js @@ +357,5 @@ > .eql(sharedUtils.CALL_TYPES.AUDIO_VIDEO); > }); > > describe("incoming calls", function() { > + var consoleErrorStub; Ditto wrt not needing to alias names in this file & the other test files (additionally some of these aliases aren't actually used). ::: browser/components/loop/test/shared/utils_test.js @@ +339,5 @@ > }); > + > + it("should log an error message to the console", function() { > + // Stub to prevent console messages > + var errorStub = sandbox.stub(window.console, "error"); Lets drop the alias, and take the stub up to a beforeEach as its used in two tests next to each other.

Comment on attachment 8631276 [details] [diff] [review] Fix all warnings in Loop shared test suite Review of attachment 8631276 [details] [diff] [review]: ----------------------------------------------------------------- That's great. I'll land this for you now since I've got some other patches that are likely to conflict with this.