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

Redis module default configuration must be more foolproof. #4229

Merged
merged 2 commits into from May 20, 2017

Conversation

hchonan
Copy link
Contributor

@hchonan hchonan commented May 19, 2017

According to Document ( http://codeception.com/docs/modules/Redis ) and comments of module source, it says Codeception flushes the database "At the beginning of every suite" ( cleanupBefore: 'suite' ) . However default setting of cleanupBefore is set to "At the beginning of every test" ( cleanupBefore: 'test' ), so description of documentation is not actually true.

And flushing database without any attention seems dangerous behavior, so I turned default configuration to safe-side.

Copy link
Member

@Naktibalda Naktibalda left a comment

Choose a reason for hiding this comment

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

Cleanup before each test is consistent with DB and AMQP modules.

Changing default value breaks tests, so it's better to merge this change to master branch so it only takes effect in 2.3.0 release (coming soon).

@DavertMik
Copy link
Member

Yes, this will be consistent with Db in 2.3.

@DavertMik DavertMik changed the base branch from 2.2 to master May 20, 2017 22:56
@DavertMik
Copy link
Member

Thanks.

Please note, this fix will be available starting from Codeception 2.3.0

@DavertMik DavertMik merged commit 9e80744 into Codeception:master May 20, 2017
@hchonan
Copy link
Contributor Author

hchonan commented May 22, 2017

Thanks for quick reaction!

@hchonan hchonan deleted the foolproof-of-redis-module branch May 22, 2017 04:07
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.

None yet

4 participants