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 for #99 -- if no time to be frozen is available use the real datetime #155

Merged
merged 1 commit into from
Jan 19, 2017

Conversation

mindojo-victor
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Sep 28, 2016

Coverage Status

Coverage increased (+0.02%) to 96.793% when pulling 9714f9d on mindojo-victor:master into a6653a3 on spulec:master.

@spulec
Copy link
Owner

spulec commented Oct 9, 2016

Is this necessary? Should we ever be calling _time_to_freeze if there isn't cls.times_to_freeze?

@mindojo-victor
Copy link
Contributor Author

Should we ever be calling _time_to_freeze if there isn't cls.times_to_freeze?

You mean to check if cls.times_to_freeze is empty in each place where we call _time_to_freeze?

@spulec
Copy link
Owner

spulec commented Oct 13, 2016

I'm trying to figure out the scenario in which this happens. Do you have a test to reproduce the issue?

@mindojo-victor
Copy link
Contributor Author

I don't have the reproducible tests. It happens sometimes. Looks like it depends on test sequence. Some tests patch the datetime module, then next test_* is module is run, but the decorator is already finished. I am using nose2.

If I were to decide what to do with this, I would not try to restore original datetime module. Just leave the module patched: if there is a frozen datetime to return, return it, otherwise return pass through to the original datetime. This would make the implementation much simpler.

@spulec spulec merged commit ac60f99 into spulec:master Jan 19, 2017
@spulec
Copy link
Owner

spulec commented Jan 19, 2017

thanks!

@AndreasBackx
Copy link

@spulec could a new release be published to Pypi that includes this and the other new changes? We're currently using the latest commit for testing because of this bug.

@spulec
Copy link
Owner

spulec commented May 13, 2017

Version 0.3.9 is now released!

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

Successfully merging this pull request may close these issues.

4 participants