Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sinon not stubbing a function in feature test #40

Open
zanenkn opened this issue Apr 28, 2019 · 6 comments
Open

Sinon not stubbing a function in feature test #40

zanenkn opened this issue Apr 28, 2019 · 6 comments

Comments

@zanenkn
Copy link

zanenkn commented Apr 28, 2019

Problem

Sinon is not stubbing my function

Background

Testing the RPS. Users choice is determined by listening for the button click. Computers choice is determined by method that picks a random between them three. Winner is determined by comparing these two. For test purposes I want to stub the computers choice - say that it is "rock" at all times. Sinon just wouldn't cooperate - tests are coming back sometimes red and sometimes green - the computers choice is still random.

Feature test

require('../spec.helper');
var sinon = require('sinon');

context('The game', () => {
  before(async () => {
    await browser.init()
    await browser.visitPage('http://localhost:8080/')  
  });

  beforeEach(async () => {
    await browser.page.reload();
  });

  after(() => {
    browser.close();
  });

  it("returns 'tie' message if user picks 'rock' and computer picks 'rock'", async () => {
    let computer = new Computer
    await sinon.stub(computer, 'choice').returns("rock");
    await browser.clickOnButton("button[id='rock']");
    let content = await browser.getContent("div[id='message']");
    expect(content).to.contain('You chose rock, computer chose rock. It is a tie.');
  });

});

Implementation code

document.addEventListener('DOMContentLoaded', () => {
    let choice = document.querySelectorAll("button[tag='choice']")
    for(var i = 0; i < choice.length; i++){

        choice[i].addEventListener("click", function() {
            event.preventDefault();
            let computer= new Computer
            let computersChoice = computer.choice();

            if (userChoice == computersChoice) {
                document.getElementById("message").innerHTML = `You chose ${userChoice}, computer chose ${computersChoice}. It is a tie.`
            } else if (
                (userChoice == "rock" && computersChoice == "scissors") ||
                (userChoice == "scissors" && computersChoice == "paper") ||
                (userChoice == "paper" && computersChoice == "rock")
            ) {
                document.getElementById("message").innerHTML = `You chose ${userChoice}, computer chose ${computersChoice}. You win!`
            } else {
                document.getElementById("message").innerHTML = `You chose ${userChoice}, computer chose ${computersChoice}. You lose!`
            }    
        })    
    }
})
function setUserChoice(choice) {
    userChoice = choice
}

class Computer {
    constructor(choice) {
        this.choice = function () {
            let num = Math.floor((Math.random() * 3) + 1)
            if (num==1) {
                return "rock"
            } else if (num==2) {
                return "paper"
            } else {return "scissors"}  
        }
    }
}

How did you try to solve the problem?

  1. Read the documentation in Sinon js

  2. Rewrote the Computer as so

const Computer = {
    choice: function () {
        let num = Math.floor((Math.random() * 3) + 1)
        if (num==1) {
            return "rock"
        } else if (num==2) {
            return "paper"
        } else {return "scissors"}
    }
}

and the stub as so

sinon.stub(Computer, 'choice').get(function getterFn() {
      return 'rock';
});

(didnt do much)

  1. Tried to stick the stub definition into the Before and Before each blocks
  2. Rewrote the stub as so
sinon.stub(Computer, 'choice').value('rock')
@zanenkn
Copy link
Author

zanenkn commented Apr 29, 2019

Ok, so.
I have come up with three ways to write stubs:
no1

let computer = new Computer
await sinon.stub(computer, 'choice').returns("rock")

no2

sinon.stub(Computer, 'choice').get(function getterFn() {
   return 'rock';
 });

no3

sinon.stub(Computer, 'choice').value('rock')

The first one works with the Computer set up like that:

class Computer {
    constructor(choice) {
        this.choice = function () {
            let num = Math.floor((Math.random() * 3) + 1)
            if (num==1) {
                return "rock"
            } else if (num==2) {
                return "paper"
            } else {return "scissors"}  
        }
    }
}

The number 2 and 3 works with this:

const Computer = {
    choice: function () {
        let num = Math.floor((Math.random() * 3) + 1)
        if (num == 1) {
            return "rock"
        } else if (num == 2) {
            return "paper"
        } else { return "scissors" }
    }
}

I have tested all the three versions (with unit tests, because TDD) and they do their job, they return the value, for example this comes back green:

it("stub1 works", () => {
    let comp = new Computer
    sinon.stub(comp, 'choice').returns("rock")
    expect(comp.choice()).to.eql("rock")
});

The problems start when I try to insert it in the feature test. It just wouldnt do it. I suspect it happens because tests in the feature tests are async? Right? Wrong? What do?

@zanenkn zanenkn closed this as completed Apr 29, 2019
@zanenkn zanenkn reopened this Apr 29, 2019
@zanenkn
Copy link
Author

zanenkn commented Apr 29, 2019

Eh, i dont know what happened, but apparently i closed the issue somehow by mistake.
This is still a problem.
Pls help.

@tochman
Copy link
Member

tochman commented Apr 29, 2019

Impressive progress. I would need to see HOW you use it in your acceptance tests. Please post an example.

@tochman
Copy link
Member

tochman commented Apr 29, 2019

Generally, if you use Math.floor(Math.random() * 3 + 1) you should be able to stub out that function.
I would make it really simple:

sinon.stub(Math, 'floor').returns(2);

Now every method that uses this will return 2

I would just throw this in into the before-block of my test and see what happens...

@zanenkn
Copy link
Author

zanenkn commented Apr 29, 2019

Heres my feature test:

require('../spec.helper');
var sinon = require('sinon');

context('The game', () => {
  before(async () => {
    await browser.init()
    await browser.visitPage('http://localhost:8080/')  
  });

  beforeEach(async () => {
    await browser.page.reload();
    sinon.stub(Math, 'floor').returns(1);
    
  });

  after(() => {
    browser.close();
  });

  it("returns 'tie' message if user picks 'rock' and computer picks 'rock'", async () => {
    await browser.clickOnButton("button[id='rock']");
    let content = await browser.getContent("div[id='message']");
    expect(content).to.contain('You chose rock, computer chose rock. It is a tie.');
  });

});

Tried this, but still get assertion errors. Tried all the three options - before block, before each block and the actual it block, also tried to add await before the sinon, but no luck. (i have tried all these things as well with my three working stubs, same result)

I mean, this is my whole thing here:

document.addEventListener('DOMContentLoaded', () => {
    let choice = document.querySelectorAll("button[tag='choice']")
    for(var i = 0; i < choice.length; i++){

        choice[i].addEventListener("click", function() {
            event.preventDefault();
            let computer= new Computer
            let computersChoice = computer.choice();

            if (userChoice == computersChoice) {
                document.getElementById("message").innerHTML = `You chose ${userChoice}, computer chose ${computersChoice}. It is a tie.`
            } else if (
                (userChoice == "rock" && computersChoice == "scissors") ||
                (userChoice == "scissors" && computersChoice == "paper") ||
                (userChoice == "paper" && computersChoice == "rock")
            ) {
                document.getElementById("message").innerHTML = `You chose ${userChoice}, computer chose ${computersChoice}. You win!`
            } else {
                document.getElementById("message").innerHTML = `You chose ${userChoice}, computer chose ${computersChoice}. You lose!`
            }    
        })    
    }
})

Can the problem be that the whole computer choice / math function is inside this monstrous event listener block?

If we look at the feature test, the flow goes like this:

  1. Sinon stubs the Computers choice to rock
  2. Browser clicks on the button and sets the whole block in motion
  3. Browser looks for the message and evaluates it

Can it be that the step 2 overrides the stub? As far as i have understood from Sinon's docs it shouldnt be the case. I mean otherwise, how are we ever supposed to stub anything? (many questions)

@zanenkn
Copy link
Author

zanenkn commented Apr 29, 2019

heres my repo: https://github.com/zanenkn/rock_paper_scissors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants