0

Ive the following code which use the local storage that working as expected and I want to create unit test for it the issue is that i use in this function the local storage

This is the function that I want to test

open: function(url) {
            var that = this;
            if (!localStorage.getItem(“alert”)) {               
                that_run(url)
                    .then(function(isBlocked) {
                        if (isBlocked) { 
                            that._notify();
                            localStorage.setItem(“alert”, true);
                        } else { 
                            localStorage.setItem(“alert”, true);
                            that._exe(“page”,url)
                        }
                    }).done();
            } else {
                that._exe(“page”,url)
            }
        },

This is the test which is working but I think to rewrite the window is bad practice and my question if I can write this test better ?

it.only("test for open", function () {
    var url = "http://mytesturl”;
    winlocalStorage.getItem = function(){
        return false;
    };
    var oSimulateSpy = sandbox.spy(exc "_simulate");
    return orun.open(url).then(function(){
        expect(oSimulateSpy.called).to.be.true;
    });
}); 

I saw this post and this use functional programing https://stackoverflow.com/a/20153543/6124024 but I think to pass the local storage as parameter is a bit overkill in this case since this function (open) is called many times from many places... is there a better/cleaner way to handle this?

Community
  • 1
  • 1
  • The suggestion on that answer it the right way to do it. You can hack it as Stuart suggested, but it's a hack, it requires that your test have intimate knowledge of implementation details. If you pass in an object that can be stubbed, it's part of your API. – Ruan Mendes Nov 07 '16 at 13:48
  • @JuanMendes - sorry I miss this comment :) Ok so this is the way to go and you also think that using here functional solution is overkill ? –  Nov 07 '16 at 13:53
  • I'm not sure what you mean by "think that using here functional solution is overkill" I also can't make sense of your question, I only see one approach being used and it doesn't seem related to functional programming. I also don't know what functional programming has to do with overrides. In any case, I can tell you that the solution suggested in the answer you posted is the standard way to do it and is not considered overkill if you believe your code must be tested. You could go further and use a dependency injection framework, but that is beyond your question. – Ruan Mendes Nov 07 '16 at 14:09
  • @JuanMendes - sorry so I not sure I understand , when you tell about the right way you mean the code which I put for the test which is currenlty working or the link that I put for other solution which is functional I guess ? –  Nov 07 '16 at 14:12
  • The key is: if you need testing, be sure to allow external dependencies to be mocked through the API, either a constructor param, or a property/method that can be overridden – Ruan Mendes Nov 07 '16 at 14:12
  • my comment says: "I can tell you that the solution suggested in the [answer you posted](http://stackoverflow.com/a/20153543/6124024) is the standard way to do it and is not considered overkill " – Ruan Mendes Nov 07 '16 at 14:13
  • @JuanMendes just to verify : ) this solution http://stackoverflow.com/questions/11485420/how-to-mock-localstorage-in-javascript-unit-tests/20153543#20153543 or my solution it.only("test for open", function () { var url = "http://mytesturl”; winlocalStorage.getItem = function(){ return false; }; var oSimulateSpy = sandbox.spy(exc "_simulate"); return orun.open(url).then(function(){ expect(oSimulateSpy.called).to.be.true; }); }); , the first or the second ? –  Nov 07 '16 at 14:15
  • I can't really parse the code in comments, but neither of them looks like you changed the API to the object being tested. You need to pass a stub implementation into your object being tested. Read the accepted answer on the post you linked to. Sorry, but you have to learn to write clearer questions. – Ruan Mendes Nov 07 '16 at 14:27

1 Answers1

1

Your struggling because your code is loaded with all sorts of side effects. Because you don't return any value from this._exe or that_run, it's obvious to me that there are side effects in those functions too. You're better off return values, or Promised values from those functions rather than relying upon those functions to change even more external state.

Here's a possible better way to write your module

// use "wrapper" function that configures this module
export default function(storage) {
  return {
    // other functions ...
    async open (url) {
      if (storage.getItem('alert')) {
        return this._exe('page', url) // make sure _exe returns promise
      }
      else if (await that_run(url)) {
        storage.setItem('alert', true)
        return this.notify() // make sure notify returns Promise
        // or return Promise.resolve()
      }
      else {
        storage.setItem('alert', true)
        return this._exe('page', url)
    }
  }
}

Using your module in testing code

// import mock storage adapter
const MockStorage = require('./mock-storage');

// create mock storage adapter
const mockStorage = new MockStorage();

// pass mock storage adapter to your module as a config argument
const myModule = require('./my-module')(mockStorage);

// don't forget to reset storage before each test
beforeEach(function() {
  mockStorage.reset();
});

it("test for open", async function () {
  var url = "http://mytesturl";
  mockStorage.set('alert', false);
  let someValue = await myModule.open(url);
  assert.equal(someValue, someExpectation);
  assert.equal(mockStorage.get('alert'), true);
});

The mock storage adapter might look like this

export default class MockStorage {
  constructor () {
    this.storage = new Map();
  }
  setItem (key, value) {
    this.storage.set(key, value);
  }
  getItem (key) {
    return this.storage.get(key);
  }
  removeItem (key) {
    this.storage.delete(key);
  }
  clear () {
    this.constructor();
  }
}

Then, when you use your module in production code, you can pass the real localStorage

// use window.localStorage as config argument in production code
const myModule = require('./my-module')(window.localStorage);
Mulan
  • 129,518
  • 31
  • 228
  • 259
  • Thanks a lot naomik 1+ but since im not familiar and cant use currenlty in my company the ES6 can you please change it to es5 that I can test it and use it? thanks sir! –  Nov 08 '16 at 20:08
  • ive also 1+ your answer in the other question with the same topic since I think this a good way! –  Nov 08 '16 at 20:10
  • Can you please assist? –  Nov 09 '16 at 12:58