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

fix timeout to allow fake timers #562

Merged
merged 6 commits into from May 30, 2016
Merged

Conversation

mjhm
Copy link
Contributor

@mjhm mjhm commented Apr 25, 2016

@charlierudolph This is the fix to allow sinon.useFakeTimers or similar time mocking to work. #535

I'm not sure exactly where to plug in new tests for it though.

I have my own example test repo at https://github.com/mjhm/cucumber-test which demonstrates the problem using 0.10.2 vs this update.

@charlierudolph
Copy link
Member

Maybe throw in a feature test that uses sinon.useFakeTimers or stubs out one of the methods?

@mjhm
Copy link
Contributor Author

mjhm commented Apr 30, 2016

@charlierudolph Added feature tests.

Then it passes


Scenario: real time is restored
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not certain this is test is really needed. The name makes me think that real time was mocked but it never was.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point of this test is to show that after running the first scenario in a faked time context (note the @fakedTime tag), then the second scenario runs with real time restored (no @fakedTime tag).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but to cucumber-js real time was never faked. To me this is just testing that you correctly ran this.clock.restore() in your after hook. Not running that doesn't effect whether or not cucumber-js works.

@charlierudolph
Copy link
Member

Thanks @mjhm. Feature tests look great. Just a couple things to cleanup and this is good to go.

@mjhm
Copy link
Contributor Author

mjhm commented May 9, 2016

Here you go @charlierudolph

@mjhm
Copy link
Contributor Author

mjhm commented May 16, 2016

Ping @charlierudolph

this.After({tags: ['@fakedTime']}, function(scenario, callback) {
this.clock.restore();
callback();
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can just omit the callback in each of these since they are synchronous. Thus you can omit scenario as well

@charlierudolph charlierudolph merged commit 0c7e21b into cucumber:master May 30, 2016
@lock
Copy link

lock bot commented Oct 25, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants